server: don't stop iterating ranges if a range is missing#48681
Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom May 11, 2020
Merged
Conversation
In cockroachdb#33585 we added a commit to not return an error when a range is msising when iterating the set of ranges for the status server. This change seems to have stopped the iteration of ranges in this case, which I don't believe was the intention. This PR is a drive-by, it's not clear when this case happens. Release note: None
Member
|
This change is |
Member
|
Ugh, thanks for catching. I'll keep this on my list of things to file as starter project, this api needs to be better |
tbg
approved these changes
May 11, 2020
Contributor
Author
|
bors r=tbg |
Contributor
Build succeeded |
vrongmeal
added a commit
to vrongmeal/cockroach
that referenced
this pull request
Jul 30, 2020
Previously, all the iterators were implemented using closures that returned a bool and an error. The behaviour of bool was dependent on the documentation, i.e., whether it represented presence of "more" elements or the iteration was "done". This was prone to errors as evident by fix in cockroachdb#48681. This change introduces a type that can be commonly used by all closure based iterators to implement them. Package iterutil declares State which defines the following listed methods: - Cur: returns the current element of the iteration - Stop: stops the iteration - Done: checks if the iteration is over - Update: sets the value of current element Done and Update methods can be used to create the iterator whereas the Cur and Stop methods can be used by the closure to get current element and stop the iteration respectively. Resolves: cockroachdb#48709 Release note: None Signed-off-by: Vaibhav <vrongmeal@gmail.com>
vrongmeal
added a commit
to vrongmeal/cockroach
that referenced
this pull request
Aug 2, 2020
Previously, all the iterators were implemented using closures that returned a bool and an error. The behaviour of bool was dependent on the documentation, i.e., whether it represented presence of "more" elements or the iteration was "done". This was prone to errors as evident by fix in cockroachdb#48681. This change introduces a type that can be commonly used by all closure based iterators to implement them. Package iterutil declares State which defines the following listed methods: - Cur: returns the current element of the iteration - Done: checks if the iteration is over - Update: sets the value of current element Now, closures can take the "cur" as a parameter to get the current element. Cur returns a wrapper which can be used to access the element associated with the state of iteration and the index. This "cur" can also be used to stop the iteration using Stop method. Resolves: cockroachdb#48709 Release note: None Signed-off-by: Vaibhav <vrongmeal@gmail.com>
vrongmeal
added a commit
to vrongmeal/cockroach
that referenced
this pull request
Aug 2, 2020
Resolves: cockroachdb#48681 Release note: None Signed-off-by: Vaibhav <vrongmeal@gmail.com>
vrongmeal
added a commit
to vrongmeal/cockroach
that referenced
this pull request
Aug 5, 2020
Previously, all the iterators were implemented using closures that returned a bool and an error. The behaviour of bool was dependent on the documentation, i.e., whether it represented presence of "more" elements or the iteration was "done". This was prone to errors as evident by fix in cockroachdb#48681. This change introduces a type that can be commonly used by all closure based iterators to implement them. Package iterutil declares State which defines the following listed methods: - Current: returns the current element of the iteration - Done: checks if the iteration is over Now, closures can take the "cur" as a parameter to get the current element. Cur returns a wrapper which can be used to access the element associated with the state of iteration and the index. This "cur" can also be used to stop the iteration using Stop method. Resolves: cockroachdb#48709 Release note: None Signed-off-by: Vaibhav <vrongmeal@gmail.com>
vrongmeal
added a commit
to vrongmeal/cockroach
that referenced
this pull request
Aug 5, 2020
Resolves: cockroachdb#48681 Release note: None Signed-off-by: Vaibhav <vrongmeal@gmail.com>
tbg
pushed a commit
to vrongmeal/cockroach
that referenced
this pull request
Aug 6, 2020
Previously, all the iterators were implemented using closures that returned a bool and an error. The behaviour of bool was dependent on the documentation, i.e., whether it represented presence of "more" elements or the iteration was "done". This was prone to errors as evident by fix in cockroachdb#48681. This change introduces a type that can be commonly used by all closure based iterators to implement them. Package iterutil declares State which defines the following listed methods: - Current: returns the current element of the iteration - Done: checks if the iteration is over Now, closures can take the "cur" as a parameter to get the current element. Cur returns a wrapper which can be used to access the element associated with the state of iteration and the index. This "cur" can also be used to stop the iteration using Stop method. Resolves: cockroachdb#48709 Release note: None Signed-off-by: Vaibhav <vrongmeal@gmail.com>
tbg
pushed a commit
to vrongmeal/cockroach
that referenced
this pull request
Aug 6, 2020
Resolves: cockroachdb#48681 Release note: None Signed-off-by: Vaibhav <vrongmeal@gmail.com>
craig Bot
pushed a commit
that referenced
this pull request
Aug 6, 2020
52158: util: add iterutil to create consistent iterators r=tbg a=vrongmeal Previously, all the iterators were implemented using closures that returned a bool and an error. The behaviour of bool was dependent on the documentation, i.e., whether it represented presence of "more" elements or the iteration was "done". This was prone to errors as evident by fix in #48681. This change introduces a type that can be commonly used by all closure based iterators to implement them. Package iterutil declares State which defines the following listed methods: - Current: returns the current element of the iteration - Done: checks if the iteration is over Now, closures can take the "cur" as a parameter to get the current element. Cur returns a wrapper which can be used to access the element associated with the state of iteration and the index. This "cur" can also be used to stop the iteration using Stop method. Resolves: #48709 Release note: None Signed-off-by: Vaibhav <vrongmeal@gmail.com> 52439: backupccl: fix race on RESTORE's summary r=pbardea a=pbardea When RESTORE would return its bulk op summary, there would be a race between returning the results in the error case and writing the bulk op summary in the progress loop. We don't need to return the most up-to-date results since we discard them anyway. Fixes #52390. Release note: None Co-authored-by: Vaibhav <vrongmeal@gmail.com> Co-authored-by: Paul Bardea <pbardea@gmail.com>
vrongmeal
added a commit
to vrongmeal/cockroach
that referenced
this pull request
Aug 20, 2020
Closure based iterators returned two values: a bool and an error. This was heavily documentation based whether the boolean meant "more" or "done". This was prone to errors as evident by cockroachdb#48681. This commit introduces a sentinel error, which means that the iteration should stop if ErrStopIteration is returned. Resolves: cockroachdb#48709 Release note: None Signed-off-by: Vaibhav <vrongmeal@gmail.com>
vrongmeal
added a commit
to vrongmeal/cockroach
that referenced
this pull request
Aug 30, 2020
Closure based iterators returned two values: a bool and an error. This was heavily documentation based whether the boolean meant "more" or "done". This was prone to errors as evident by cockroachdb#48681. This commit introduces a sentinel error, which means that the iteration should stop if errStopIteration is returned. Resolves: cockroachdb#48709 Release note: None Signed-off-by: Vaibhav <vrongmeal@gmail.com>
tbg
pushed a commit
to vrongmeal/cockroach
that referenced
this pull request
Oct 7, 2020
Closure based iterators returned two values: a bool and an error. This was heavily documentation based whether the boolean meant "more" or "done". This was prone to errors as evident by cockroachdb#48681. This commit introduces a sentinel error, which means that the iteration should stop if errStopIteration is returned. Resolves: cockroachdb#48709 Release note: None Signed-off-by: Vaibhav <vrongmeal@gmail.com>
jayshrivastava
pushed a commit
that referenced
this pull request
Oct 8, 2020
Closure based iterators returned two values: a bool and an error. This was heavily documentation based whether the boolean meant "more" or "done". This was prone to errors as evident by #48681. This commit introduces a sentinel error, which means that the iteration should stop if errStopIteration is returned. Resolves: #48709 Release note: None Signed-off-by: Vaibhav <vrongmeal@gmail.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
In #33585 we added a commit to not return an error when a range is msising
when iterating the set of ranges for the status server. This change seems
to have stopped the iteration of ranges in this case, which I don't believe
was the intention.
This PR is a drive-by, it's not clear when this case happens.
Release note: None