Add common blocking implementation details to docs #5358
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These come up over and over again with blocking query loops in our own code and third-party's. #5333 is possibly a case (unconfirmed) where "badly behaved" blocking clients cause issues, however since we've never explicitly documented these things it's not reasonable for third-party clients to have guessed that they are needed! This hopefully gives us something to point to for the future. It's a little wordy - happy to consider breaking some of the blocking stuff out of this page if we think it's appropriate but just wanted to quickly plaster over this gap in our docs for now.
lkysow
reviewed
Feb 19, 2019
kaitlincart
added
the
type/docs
kaitlincart
reviewed
Feb 19, 2019
kaitlincart
reviewed
Feb 19, 2019
kaitlincart
reviewed
Feb 19, 2019
kaitlincart
reviewed
Feb 19, 2019
rboyer
reviewed
Feb 19, 2019
rboyer
reviewed
Feb 19, 2019
Co-Authored-By: banks <banks@banksco.de>
@lkysow I did consider that. The consul built in watch package is actually the most correct example but it's also not super easy to read because it has a few abstractions in the way. Consul template's implementation actually doesn't do all of the recommended things here and the ones it does are buried in a few places under some other abstractions! We have a few more recently written blocking loops within Consul code that more closely follow this pattern and are a bit simpler to read but ultimately it seemed easier to just describe the ideal case. |
👍 |
|
@kaitlincarter-hc done, do you think this is good to merge and release? Feel free if you have time otherwise I'll get to it. |
kaitlincart
reviewed
Feb 20, 2019
kaitlincart
reviewed
Feb 20, 2019
kaitlincart
pushed a commit
that referenced
this pull request
Feb 21, 2019
* Add common blocking implementation details to docs These come up over and over again with blocking query loops in our own code and third-party's. #5333 is possibly a case (unconfirmed) where "badly behaved" blocking clients cause issues, however since we've never explicitly documented these things it's not reasonable for third-party clients to have guessed that they are needed! This hopefully gives us something to point to for the future. It's a little wordy - happy to consider breaking some of the blocking stuff out of this page if we think it's appropriate but just wanted to quickly plaster over this gap in our docs for now. * Update index.html.md * Apply suggestions from code review Co-Authored-By: banks <banks@banksco.de> * Update index.html.md * Update index.html.md * Clearified monotonically * Fixing formating
johncowen
pushed a commit
that referenced
this pull request
Mar 11, 2019
More recommendations for blocking queries clients was added here: #5358 This commit mainly adds cursor/index validation/correction based on these recommendations (plus tests) The recommendations also suggest that clients should include rate limiting. Because of this, we've moved the throttling out of Consul UI specific code and into Blocking Query specific code. Currently the 'rate limiting' in this commit only adds a sleep to every iteration of the loop, which is not the recommended approach, but the code here organizes the throttling functionality into something we can work with later to provide something more apt.
johncowen
added a commit
that referenced
this pull request
Mar 22, 2019
…5470) More recommendations for blocking queries clients was added here: #5358 This commit mainly adds cursor/index validation/correction based on these recommendations (plus tests) The recommendations also suggest that clients should include rate limiting. Because of this, we've moved the throttling out of Consul UI specific code and into Blocking Query specific code. Currently the 'rate limiting' in this commit only adds a sleep to every iteration of the loop, which is not the recommended approach, but the code here organizes the throttling functionality into something we can work with later to provide something more apt.
johncowen
added a commit
that referenced
this pull request
Apr 29, 2019
…5470) More recommendations for blocking queries clients was added here: #5358 This commit mainly adds cursor/index validation/correction based on these recommendations (plus tests) The recommendations also suggest that clients should include rate limiting. Because of this, we've moved the throttling out of Consul UI specific code and into Blocking Query specific code. Currently the 'rate limiting' in this commit only adds a sleep to every iteration of the loop, which is not the recommended approach, but the code here organizes the throttling functionality into something we can work with later to provide something more apt.
johncowen
added a commit
that referenced
this pull request
May 1, 2019
…5470) More recommendations for blocking queries clients was added here: #5358 This commit mainly adds cursor/index validation/correction based on these recommendations (plus tests) The recommendations also suggest that clients should include rate limiting. Because of this, we've moved the throttling out of Consul UI specific code and into Blocking Query specific code. Currently the 'rate limiting' in this commit only adds a sleep to every iteration of the loop, which is not the recommended approach, but the code here organizes the throttling functionality into something we can work with later to provide something more apt.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These come up over and over again with blocking query loops in our own code and third-party's. #5333 is possibly a case (unconfirmed) where "badly behaved" blocking clients cause issues, however since we've never explicitly documented these things it's not reasonable for third-party clients to have guessed that they are needed!
This hopefully gives us something to point to for the future.
It's a little wordy - happy to consider breaking some of the blocking stuff out of this page if we think it's appropriate but just wanted to quickly plaster over this gap in our docs for now.