-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-28055 Performance improvement for scan over several stores. #5379
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
Conversation
No new UT is required. The one for HBASE-19863 works just fine with the problem. |
💔 -1 overall
This message was automatically generated. |
retest |
💔 -1 overall
This message was automatically generated. |
Test failure is not related to the patch and it passed locally. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per Sergey explanation in private chat (copying below) , it seems cell.getTimestamp() == PrivateConstants.OLDEST_TIMESTAMP is enough to regard the cell as fake
There was an assumption that compareKeyForNextColumn would tell us whether the nextCell lower than the current cell. But the result of compareKeyForNextColumn is based on the fact of availability of the next column. So, to get the expected result the regular comparison of cell and nextCell should be used. At the same time the situation when nextCell is lower than the current cell happens only when the current cell is artificial (fake) one, so the 'official' way to do that is to check the timestamp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, basically this change shortcut part of the function of matcher.compareKeyForNextColumn
and compare the LastOnRowCell
with the HConstants.OLDEST_TIMESTAMP
before we can skip to the next column
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
…res. (apache#5379)" This reverts commit 4c1fd0d.
My bad , I will create PRs for backport next time |
…ache#5379) Signed-off-by: Ankit Singhal <ankit@apache.org> Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org>
No description provided.