-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
[#3388]Remove the deleted roles and permissions info during reloading role info #3419
Conversation
junyan29 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Why do not clear roleSet, roleInfoMap and permissionInfoMap before putAll/addAll? |
license/cla Pending — Contributor License Agreement is not signed yet. |
The reason is your git username and email is not your github id. It's different. Solution is to set the git username and email to your github id. Search in search engine, you can get many answer. |
And for the PR content, I describe in PR before you submit. Why don't clear all map and then putAll again, but to remove one by one. I think the new put is all data for users and permissions. @chuntaojun Is my understanding correct? |
The clear method cannot guarantee the whole logic as a atomic operation. |
I think this is too complicated, but logically, there are only reads to the map, not writes. You can take advantage of the volatile property, replace the application directly, and then destroy the old Map |
I agree with @chuntaojun, if you think about the thread safe, you can use a tmp map/set to save new data from persistence, and directly replace them. |
Once more about the CLA, I check your commit log, it contain many |
Thanks for your reply. |
If these map/set need to be reset, remove |
agree
发自我的iPhone
在 2020年7月24日,17:03,杨翊 SionYang <[email protected]> 写道:
If these map/set need to be reset, remove final will be ok. @chuntaojun<https://github.com/chuntaojun> What's your idea?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#3419 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AFS35NEZJPDWPAT7FIIMPLTR5FE7HANCNFSM4PFI2W6Q>.
|
Closed it and recreate a new PR([#3388]Reset nacos roles, permissions during reloading role info from database) to fix issue 3388 since CLA problem. |
Please do not create a Pull Request without creating an issue first.
What is the purpose of the change
#3388
Brief changelog
We should remove these already deleted roles, permissions during reloading nacos role info.
Verifying this change
XXXX
Follow this checklist to help us incorporate your contribution quickly and easily:
[ISSUE #123] Fix UnknownException when host config not exist
. Each commit in the pull request should have a meaningful subject line and body.mvn -B clean package apache-rat:check findbugs:findbugs -Dmaven.test.skip=true
to make sure basic checks pass. Runmvn clean install -DskipITs
to make sure unit-test pass. Runmvn clean test-compile failsafe:integration-test
to make sure integration-test pass.