-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-22710 Wrong result in one case of scan that use raw and version… #767
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
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
@openinx Could you help to review it? |
|
💔 -1 overall
This message was automatically generated. |
|
@bsglz , would you mind rebasing your patch on the newest version of TestStoreScanner? |
9c3f8bf to
3630a77
Compare
…s and filter together
|
rebased. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
virajjasani
left a comment
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.
Few minor comments
...server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/ScanQueryMatcher.java
Outdated
Show resolved
Hide resolved
...server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/ScanQueryMatcher.java
Show resolved
Hide resolved
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java
Outdated
Show resolved
Hide resolved
…s and filter together
|
🎊 +1 overall
This message was automatically generated. |
virajjasani
left a comment
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.
+1, pending QA. Let's wait for other reviews too.
|
Please review @yangzhe1991 @Apache9 |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
saintstack
left a comment
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.
Makes sense to me. Thanks for the patch @bsglz
If good by you @virajjasani , I can merge.
|
Thanks very much to all of the reviewers. |
Apache9
left a comment
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.
IIRC this has been discussed several times in the past, that whether we should suppor filtering when using raw scan.
The problem here is that, for a raw scan, the delete markers will also be returned, and this may cause strange behavior for some filters...
I have skimed the discuss in HBASE-16322, so finaly, as you mentioned, "we do have some usages which use raw scan and filter together", |
|
@saintstack Could you merge it, thanks. |
|
Merged the PR. Thanks @bsglz |
#767) Signed-off-by: stack <[email protected]> Signed-off-by: Duo Zhang <[email protected]> Signed-off-by: Viraj Jasani <[email protected]>
#767) Signed-off-by: stack <[email protected]> Signed-off-by: Duo Zhang <[email protected]> Signed-off-by: Viraj Jasani <[email protected]>
|
Thanks very much. |
#767) Signed-off-by: stack <[email protected]> Signed-off-by: Duo Zhang <[email protected]> Signed-off-by: Viraj Jasani <[email protected]>
apache#767) Signed-off-by: stack <[email protected]> Signed-off-by: Duo Zhang <[email protected]> Signed-off-by: Viraj Jasani <[email protected]>
apache#767) Signed-off-by: stack <[email protected]> Signed-off-by: Duo Zhang <[email protected]> Signed-off-by: Viraj Jasani <[email protected]> (cherry picked from commit 4c42d1b) Change-Id: I829d21551b625ab6217e4b10c12dcae0a2a327b9
…s and filter together