-
Notifications
You must be signed in to change notification settings - Fork 1k
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
_changes?since=X results are incorrectly ordered after two rounds of shard splitting #4640
Comments
decoded the last three;
|
Thanks for creating the issue @jcoglan !
How did we determine they are out of order? Is that based on the
I think the parser tool is ignoring an important
We notice how In general looking at the It would still be a nice minor ergonomic fix but so far nothing here violates correctness, we're not throwing changes away or returning them out of order. |
Ah, that's good to know that it works after one round of splitting.
So rewinding after two rounds of splitting is a more interesting issue. We may want make two separate issues: 1) make the |
When we get sequence before the split, we'll fill in the missing (now split) ranges with a special `{split, OldNodeUUid}` marker. However, when sequences are emitted in the changes API, that will make the N- prefix (SeqSum) bounce around from higher to lower numbers, while users expect those to be mostly incrementing. So take a conservative approach and assume it will be full rewind for that ramge, and use 0 for that range. This is a purely cosmetic thing, when we decode the sequence that prefix gets thrown away anyway. Fixes a part of issue: #4640
This should fix the apparent incorrect order #4643 |
When we get sequence before the split, we'll fill in the missing (now split) ranges with a special `{split, OldNodeUUid}` marker. However, when sequences are emitted in the changes API, that will make the N- prefix (SeqSum) bounce around from higher to lower numbers, while users expect those to be mostly incrementing. So take a conservative approach and assume it will be a full rewind for that range, and use 0 instead. This is a purely cosmetic thing, when we decode the sequence that prefix gets thrown away anyway. To emphasize that the sequence prefix is mostly a visual aid, rename the seq/1 function to fake_packed_seq/1 with a comment about what it does and a note about the special split case. Fixes a part of issue: #4640
When we get sequence before the split, we'll fill in the missing (now split) ranges with a special `{split, OldNodeUUid}` marker. However, when sequences are emitted in the changes API, that will make the N- prefix (SeqSum) bounce around from higher to lower numbers, while users expect those to be mostly incrementing. So take a conservative approach and assume it will be a full rewind for that range, and use 0 instead. This is a purely cosmetic thing, when we decode the sequence that prefix gets thrown away anyway. To emphasize that the sequence prefix is mostly a visual aid, rename the seq/1 function to fake_packed_seq/1 with a comment about what it does and a note about the special split case. Fixes a part of issue: #4640
When we get sequence before the split, we'll fill in the missing (now split) ranges with a special `{split, OldNodeUUid}` marker. However, when sequences are emitted in the changes API, that will make the N- prefix (SeqSum) bounce around from higher to lower numbers, while users expect those to be mostly incrementing. So take a conservative approach and assume it will be a full rewind for that range, and use 0 instead. This is a purely cosmetic thing, when we decode the sequence that prefix gets thrown away anyway. To emphasize that the sequence prefix is mostly a visual aid, rename the seq/1 function to fake_packed_seq/1 with a comment about what it does and a note about the special split case. Fixes a part of issue: #4640
When we get sequence before the split, we'll fill in the missing (now split) ranges with a special `{split, OldNodeUUid}` marker. However, when sequences are emitted in the changes API, that will make the N- prefix (SeqSum) bounce around from higher to lower numbers, while users expect those to be mostly incrementing. So take a conservative approach and assume it will be a full rewind for that range, and use 0 instead. This is a purely cosmetic thing, when we decode the sequence that prefix gets thrown away anyway. To emphasize that the sequence prefix is mostly a visual aid, rename the seq/1 function to fake_packed_seq/1 with a comment about what it does and a note about the special split case. Fixes a part of issue: #4640
When we get sequence before the split, we'll fill in the missing (now split) ranges with a special `{split, OldNodeUUid}` marker. However, when sequences are emitted in the changes API, that will make the N- prefix (SeqSum) bounce around from higher to lower numbers, while users expect those to be mostly incrementing. So take a conservative approach and assume it will be a full rewind for that range, and use 0 instead. This is a purely cosmetic thing, when we decode the sequence that prefix gets thrown away anyway. To emphasize that the sequence prefix is mostly a visual aid, rename the seq/1 function to fake_packed_seq/1 with a comment about what it does and a note about the special split case. Fixes a part of issue: #4640
Description
We have observed that if we capture a
seq
value from a_changes
response, and then perform two rounds of shard splitting on a database, requesting_changes?since=X
withX
set to the capturedseq
value returns results out of order; events from the same shard are not presented in ascending order. This might affect the behaviour of replicators/indexers relying onseq
values for checkpointing.Steps to Reproduce
The following script reproduces the behaviour on my machine:
The first two
_changes
requests return an emptyresults
array as expected, showing that aseq
value continues to work after one round of splitting. The final one, where aseq
captured whileq=2
is used whenq=8
returns the entire history, with events out of order.For example, on one run of this script the
_changes
response contained:The descending value of the numeric part of the
seq
value looked curious to me so I ran these values through a parser tool and obtained:The second event has a lower sequence number for shards
20-3f
and60-7f
, showing events from some shards are being returned out of order.Using the
seq
values in this response also returned events out of order, for example requesting/test-db/_changes?since=30-g1AAAAKVeJzLYWBg4MhgTmEQTM4vTc5ISXLIyU9OzMnILy7JAUoxJTIkyf___z8rgzmRJRcowJ6alGySlmqJTQMeY5IUgGSSPcykDKYUBtbigpzMErCZpslJZmbmBqSa6QAyMx5qJgPYJAPLRAsjcwtSTUoAmVRPVdflsQBJhgYgBTR2PshcNjRzzUxTTSzMU8gydwHE3P0InxsZJ5obmpuSZdoBiGn3qe3KBxBz_5NobhYAREXOrQ
returns some out of order events:Expected Behaviour
Results in the
_changes
response should be ordered such that events from the same shard are presented in ascending order. Ideally, thesince
value would continue to limit the results returned, instead of returning the entire history.Your Environment
CouchDB info:
The text was updated successfully, but these errors were encountered: