-
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-22622 - WALKey Extended Attributes #352
Conversation
💔 -1 overall
This message was automatically generated. |
Pushed fixes for checkstyle nits. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
@apurtell @saintstack @Apache9 - adding extended attributes to WALKey as discussed in HBASE-22622. I wasn't able to use a protobuf map class because the "public" protobuf version doesn't support it so it wouldn't be backwards compatible, so I went back to @apurtell 's original suggestion of a repeated key/value attribute. Keys are strings rather than bytes because Java uses reference equality for hashmaps with byte[] keys, and it seemed better than littering the code with ByteBuffers or ImmutableBytesWritable everywhere. I've fixed the checkstyle issues, and the test failures appear to be flapping replication tests (each run gives different results even when only trivial formatting changes have been made) Related coprocessor changes will be in HBASE-22623, unless the community feels it would be better to consolidate the two patches. Thanks for taking a look! |
💔 -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.
@gjacoby126 It seems that the change doesn't compile. Could you please take a look at it?
@HorizonNet - rebased on master, and was able to build successfully locally. Hopefully now that I've pushed again that will fix it. |
🎊 +1 overall
This message was automatically generated. |
/** | ||
* Returns a map of all extended attributes injected into this WAL key. | ||
*/ | ||
default Map<String, byte[]> getExtendedAttributes() { |
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.
Missing setters?
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.
@apurtell - WALKey's interface comments specifically say that setters aren't permitted. They're meant to be immutable, so the extended attributes will be set during construction of the WALKeyImpl
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Test failures in latest precommit look environmental in nature. Will check locally before merging this. |
if (walKey.getExtendedAttributesCount() > 0){ | ||
this.extendedAttributes = new HashMap<>(walKey.getExtendedAttributesCount()); | ||
for (WALProtos.Attribute attr : walKey.getExtendedAttributesList()){ | ||
extendedAttributes.put(attr.getKey(), attr.getValue().toByteArray()); |
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.
Noticed while backporting. Attribute values may have been compressed and need to be decompressed. Will fix up for commit.
Signed-off-by: Andrew Purtell <apurtell@apache.org> Co-authored-by: Andrew Purtell <apurtell@apache.org>
Signed-off-by: Andrew Purtell <apurtell@apache.org>
Signed-off-by: Andrew Purtell <apurtell@apache.org>
Signed-off-by: Andrew Purtell <apurtell@apache.org>
No description provided.