-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
HBASE-26783 ScannerCallable doubly clears meta cache on retries #4147
Conversation
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
3c95190
to
051fe06
Compare
I hit an unrelated bug and some extra complexity for reverse scans. I filed HBASE-26790 to track the bug, since that really only affected a test but should be fixed separately. I just pushed some changes to fix reverse scans. The crux of the reverse scan issue is that it does some special magic to find the appropriate region location when using an empty start row. When a throwable is handled, we'd pass the empty row into updateRegionLocations and end up not updating anything. So we really need to update the row key for the found location, so I stashed that in the object member variable. |
🎊 +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. |
💔 -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. |
🎊 +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. |
I have 3 +1's from the build jobs here. Looking at the details of the check failure, it shows 1 test failure -- |
Actually I realized I missed some tests, so will be adding those shortly |
💔 -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. |
🎊 +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. |
🎊 +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. |
40d2c23
to
911de1e
Compare
🎊 +1 overall
This message was automatically generated. |
911de1e
to
8b8fb94
Compare
Just force pushed a no-op to try to kick off the build again... Last failure was due to space issues on CI host |
Signed-off-by: Andrew Purtell <[email protected]>
Signed-off-by: Andrew Purtell <[email protected]>
…e on retries (apache#4147) Signed-off-by: Andrew Purtell <[email protected]>
…e on retries (apache#4147) Signed-off-by: Andrew Purtell <[email protected]>
…he#4147) Signed-off-by: Andrew Purtell <[email protected]> (cherry picked from commit 9849d3c) Change-Id: I53f5d3b2faa307f21d5afe0b93b28bc8c44403f7
I renamed the protected
getRegionLocations
method so that it's clear that it is meant to be used in the prepare method, as mentioned in the new javadoc as well.I had to handle some complexity in the ReverseScannerCallable due to how it searches for the correct region location. Previously, the default RegionServerCallable.throwable call would not actually clear the cache, because the passed in
getRow()
value would be an empty row key in some cases which doesn't actually match anything in the cache. With my changes now, we keep track of the "locationSearchKey" which is the key used to find the actual region location in prepare. This waythrowable()
will be sure to clear the cache for the location we were reading from.I've added some tests here: