paginator: fix tokenizer comparison of composite index and ID#25792
paginator: fix tokenizer comparison of composite index and ID#25792chrisroberts merged 2 commits intomainfrom
Conversation
The `CreateIndexAndIDTokenizer` creates a composite token by combining the create index value and ID from the object with a `.`. Tokens are then compared lexicographically. The comparison is appropriate for the ID segment of the token, but it is not for the create index segement. Since the create index values are stored with numeric ordering, using a lexicographical comparison can cause unexpected results. For example, when comparing the token `12.object-id` to `102.object-id` the result will show `12.object-id` being greater. This is the correct comparison but it is incorrect for the intention of the token. With the knowledge of the composition of the token, the response should be that `12.object-id` is less. The unexpected behavior can be seen when performing lists (like listing allocations). The behavior is encountered inconsistently due to two requirements which must be met: 1. Create index values with a large enough span (ex: 12 and 102) 2. Correct per page value to get a "bad" next token (ex: prefix with 102) To prevent the unexpected behavior, the target token is split and the components are used individually to compare against the object. Fixes #25435
021313f to
db360fc
Compare
| @@ -0,0 +1,3 @@ | |||
| ```release-note:bug | |||
| paginator: fix tokenizer comparison of composite index and ID | |||
There was a problem hiding this comment.
This is a good commit message title, but for the changelog we try to describe this in user-visible terms. (If it weren't user-visible, we wouldn't have a changelog entry for it.) So something like:
api: Fixed a bug in pagination when there were more than 100 items in the set
|
@chrisroberts I've added the backport label for 1.10.x, but as noted in the sidebar discussion in Slack this isn't going to merge cleanly for 1.9.x+ent and 1.8.x+ent, so up to you as to whether you just want to do that by hand as a separate changset (I'd probably go for that just to avoid having to mess with backport assistant if it's not going to help). |
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Description
The
CreateIndexAndIDTokenizercreates a composite token bycombining the create index value and ID from the object with
a
.. Tokens are then compared lexicographically. The comparisonis appropriate for the ID segment of the token, but it is not for
the create index segement. Since the create index values are stored
with numeric ordering, using a lexicographical comparison can cause
unexpected results.
For example, when comparing the token
12.object-idto102.object-idthe result will show
12.object-idbeing greater. This is thecorrect comparison but it is incorrect for the intention of the token.
With the knowledge of the composition of the token, the response
should be that
12.object-idis less.The unexpected behavior can be seen when performing lists (like listing
allocations). The behavior is encountered inconsistently due to
two requirements which must be met:
To prevent the unexpected behavior, the target token is split
and the components are used individually to compare against the
object.
Testing & Reproduction steps
Example
A simple script was used to cycle through page sizes, listing allocations and checking for duplicate objects. It displays where the comparison results in unexpected behavior:
Links
#25435
Contributor Checklist
changelog entry using the
make clcommand.ensure regressions will be caught.
and job configuration, please update the Nomad website documentation to reflect this. Refer to
the website README for docs guidelines. Please also consider whether the
change requires notes within the upgrade guide.
Reviewer Checklist
backporting document.
in the majority of situations. The main exceptions are long-lived feature branches or merges where
history should be preserved.
within the public repository.