-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-26777 BufferedDataBlockEncoder$OffheapDecodedExtendedCell.deepC… #4139
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
…lone throws UnsupportedOperationException Change-Id: I45807821ebf4ae867b5c9c10e2700ecd3c009a4b
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Change-Id: I74fb38ad0dc45e9a808c06923f1f168576e7b5f4
🎊 +1 overall
This message was automatically generated. |
Change-Id: I7d12f6b03481f7ba0091af5b937b8d79136db452
🎊 +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.
Was just talking to Istvan, and his fix makes sense to me.
We could implement deepClone()
as Andrew suggested, but the current lifetime of the offheap buffers are tied to RegionScanner which we don't have here (to track up until the contents are written back out to the client). Meaning, we can't easily keep this buffer offheap at that RPC layer.
Instead, I think it makes sense to fix this method such that we don't throw an UnsupportedOperationException for data block encoded tables, and separately come back to fix the fully off-heap data access path later (rather than fix the bug and improve the off-heap data management).
🎊 +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've re-ran TestLruAdaptiveBlockCache locally a few times, and it failed once, but passed three times. |
@stoty Yes TestLruAdaptiveBlockCache is a known flaky test. |
If there are no concerns I will merge this tomorrow back to branch-2.4 |
#4139) Signed-off-by: Andrew Purtell <apurtell@apache.org>
#4139) Signed-off-by: Andrew Purtell <apurtell@apache.org>
#4139) Signed-off-by: Andrew Purtell <apurtell@apache.org>
…ExtendedCell.deepC… (apache#4139) Signed-off-by: Andrew Purtell <apurtell@apache.org>
apache#4139) Signed-off-by: Andrew Purtell <apurtell@apache.org> (cherry picked from commit 622c436) Change-Id: Ic3a4acb79a540175a42b37940ed5f5baf7a1b37c
…lone throws UnsupportedOperationException
Change-Id: I45807821ebf4ae867b5c9c10e2700ecd3c009a4b