Query pager can try to get keys from empty vector of result::partitions #7263
Assignees
Comments
avikivity
pushed a commit
that referenced
this issue
Sep 21, 2020
Before updating the _last_[cp]key (for subsequent .fetch_page()) the pager checks is 'if the pager is not exhausted OR the result has data'. The check seems broken: if the pager is not exhausted, but the result is empty the call for keys will unconditionally try to reference the last element from empty vector. The not exhausted condition for empty result can happen if the short_read is set, which, in turn, unconditionally happens upon meeting partition end when visiting the partition with result builder. The correct check should be 'if the pager is not exhausted AND the result has data': the _last_[pc]key-s should be taken for continuation (not exhausted), but can be taken if the result is not empty (has data). fixes: #7263 tests: unit(dev), but tests don't trigger this corner case Signed-off-by: Pavel Emelyanov <xemul@scylladb.com> Message-Id: <20200921124329.21209-1-xemul@scylladb.com>
|
Backported to 4.1, 4.2 (4.3 already had it). |
avikivity
pushed a commit
that referenced
this issue
Nov 29, 2020
Before updating the _last_[cp]key (for subsequent .fetch_page()) the pager checks is 'if the pager is not exhausted OR the result has data'. The check seems broken: if the pager is not exhausted, but the result is empty the call for keys will unconditionally try to reference the last element from empty vector. The not exhausted condition for empty result can happen if the short_read is set, which, in turn, unconditionally happens upon meeting partition end when visiting the partition with result builder. The correct check should be 'if the pager is not exhausted AND the result has data': the _last_[pc]key-s should be taken for continuation (not exhausted), but can be taken if the result is not empty (has data). fixes: #7263 tests: unit(dev), but tests don't trigger this corner case Signed-off-by: Pavel Emelyanov <xemul@scylladb.com> Message-Id: <20200921124329.21209-1-xemul@scylladb.com> (cherry picked from commit 550fc73)
avikivity
pushed a commit
that referenced
this issue
Nov 29, 2020
Before updating the _last_[cp]key (for subsequent .fetch_page()) the pager checks is 'if the pager is not exhausted OR the result has data'. The check seems broken: if the pager is not exhausted, but the result is empty the call for keys will unconditionally try to reference the last element from empty vector. The not exhausted condition for empty result can happen if the short_read is set, which, in turn, unconditionally happens upon meeting partition end when visiting the partition with result builder. The correct check should be 'if the pager is not exhausted AND the result has data': the _last_[pc]key-s should be taken for continuation (not exhausted), but can be taken if the result is not empty (has data). fixes: #7263 tests: unit(dev), but tests don't trigger this corner case Signed-off-by: Pavel Emelyanov <xemul@scylladb.com> Message-Id: <20200921124329.21209-1-xemul@scylladb.com> (cherry picked from commit 550fc73)
asias
pushed a commit
to asias/scylla
that referenced
this issue
Apr 15, 2021
Before updating the _last_[cp]key (for subsequent .fetch_page()) the pager checks is 'if the pager is not exhausted OR the result has data'. The check seems broken: if the pager is not exhausted, but the result is empty the call for keys will unconditionally try to reference the last element from empty vector. The not exhausted condition for empty result can happen if the short_read is set, which, in turn, unconditionally happens upon meeting partition end when visiting the partition with result builder. The correct check should be 'if the pager is not exhausted AND the result has data': the _last_[pc]key-s should be taken for continuation (not exhausted), but can be taken if the result is not empty (has data). fixes: scylladb#7263 tests: unit(dev), but tests don't trigger this corner case Signed-off-by: Pavel Emelyanov <xemul@scylladb.com> Message-Id: <20200921124329.21209-1-xemul@scylladb.com> (cherry picked from commit 550fc73)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Installation details
Scylla version (or git commit hash): 208a721
The text was updated successfully, but these errors were encountered: