-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-26001 When turn on access control, the cell level TTL of Increment and Append operations is invalid #3385
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
…ent and Append operations is invalid
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
It seems that the test failure is not related to the change. |
|
Any related unit tests? |
Reidddddd
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.
Better to add one UT
|
Have added a related unit test. |
|
🎊 +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. |
| .collect(Collectors.toList()); | ||
| } | ||
| } | ||
| } |
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.
new line?
| createTableWithCoprocessor(tableName, ChangeCellWithACLTagObserver.class.getName()); | ||
| try (Table table = connection.getTable(tableName)) { | ||
| // Append without TTL | ||
| Append firstAppend = new Append(ROW).addColumn(CF1_BYTES, CQ2, VALUE).setACL(new HashMap<>()); |
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.
Is it possible we add a non-null empty acls? then after the 2nd increment, we verify whether the result has the carried-forward acls?
|
🎊 +1 overall
This message was automatically generated. |
|
Please also pay attention to the checkstyle warnings. |
|
🎊 +1 overall
This message was automatically generated. |
| assertEquals(2, Bytes.toLong(result.getValue(CF1_BYTES, CQ1))); | ||
|
|
||
| // Wait 2s to let the second increment expire | ||
| Thread.sleep(2000); |
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.
To improve the stability of the UT, I recommend to set the sleep time higher.
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.
Increase the sleep time to 10s.
|
🎊 +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. |
| assertTrue(Bytes.equals(VALUE2, result.getValue(CF1_BYTES, CQ2))); | ||
|
|
||
| // Wait 2s to let the second append expire | ||
| Thread.sleep(2000); |
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.
Ditto.
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.
Increased the sleep time to 10s
Reidddddd
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. Let's wait for the QA result
|
🎊 +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. |
https://issues.apache.org/jira/projects/HBASE/issues/HBASE-26001