Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rpk debug bundle: Include offset from NTP response, retry if query fails #2841

Merged
merged 1 commit into from
Nov 2, 2021

Conversation

0x5d
Copy link
Contributor

@0x5d 0x5d commented Nov 1, 2021

Add retries (3) for the NTP query, and include the reported offset and local time at the time the query was issued.

Fix #2802

@0x5d 0x5d requested a review from jcsp November 1, 2021 14:14
@0x5d 0x5d self-assigned this Nov 1, 2021
@0x5d 0x5d requested review from LenaAn and twmb as code owners November 1, 2021 14:14
Copy link
Contributor

@emaxerrno emaxerrno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm.

return err
}

err = retry.Do(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be faster if we use ntp.QueryWithOptions(host, ntp.QueryOptions{Timeout: time.Second}) and query a few hosts simultaneously (or the same host a few times simultaneously) and use the first successful response.

This retry loop is simple but does mean a 3s delay if there's some problems. Also I'd maybe use a for loop directly.

But this does solve the immediate issue, which is good, and any refinement can come later if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@twmb good point. However, the whole program's minimum execution time is bound by top -b -n 10 -H -d 1, which does 10 iterations with a 1-second delay between each iteration.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh good point, I forgot about the top delay. Perfect then! :)

@0x5d 0x5d merged commit e0ffec7 into redpanda-data:dev Nov 2, 2021
@0x5d 0x5d deleted the rpk-debug-bundle-ntp branch November 2, 2021 23:56
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rpk debug bundle sometimes fails to query NTP source (failure in test_debug_bundle)
4 participants