Fix acl map num #33246
Fix acl map num #33246
Conversation
robot-clickhouse
added
the
pr-bugfix
alesapin
added
the
can be tested
| if (acl_to_num.count(acls)) | ||
| return acl_to_num[acls]; | ||
|
|
||
| /// Start from one | ||
| auto index = acl_to_num.size() + 1; | ||
| auto index = max_acl_id++; |
There was a problem hiding this comment.
Can you describe the issue in details? As far as I can see we will restore acl_to_num and num_to_acl from logs/snapshot and everything should work well. Why it will erase?
There was a problem hiding this comment.
It will erase from removeUsage method.
There was a problem hiding this comment.
But why it's a problem? If nobody uses some id from map than we can reuse it again?
There was a problem hiding this comment.
Because after erasing, the size will be reduced. When a new acls comes in, it will be assigned an index equal to the current maximum index so that the original acls will be overwritten.
There was a problem hiding this comment.
But an ID can be erased from map only when nobody use this ID and it should be safe to reuse it?
There was a problem hiding this comment.
I add some tests to be more intuitive.
|
Clear to me. We will get an issue when one ACL is removed and new added. |
|
@nicelulu Thank you. |
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix ACLMap num, because acl_to_num will erase.