Remove unnecessary locks on DiscoveryNodeManager #19843
Conversation
XuPengfei-1020
force-pushed
the
master
branch
2 times, most recently
from
e842af5 to
1a3f5b0
Compare
XuPengfei-1020
force-pushed
the
master
branch
from
1a3f5b0 to
0ab403e
Compare
| @@ -343,7 +343,7 @@ public Set<InternalNode> getNodes(NodeState state) | |||
| } | |||
|
|
|||
| @Override | |||
| public synchronized Set<InternalNode> getActiveCatalogNodes(CatalogHandle catalogHandle) | |||
| public Set<InternalNode> getActiveCatalogNodes(CatalogHandle catalogHandle) | |||
There was a problem hiding this comment.
Reasonably configured intellij will now produce a warning
Access to field 'activeNodesByCatalogHandle' outside of declared guards
There was a problem hiding this comment.
Fixed, remove the @GuardedBy('this') modifier on the field 'activeNodesByCatalogHandle'.
|
Under what circumstances does this change make a difference? |
Threads which try to 'getAllNode' or 'getActiveConnectorNodes' could be against with thread that are refreshing workers state. This control of race condition seems unnecessary, so removing locks may be improve performance slightly.
These methods with lock are simple enough, so remove the lock on it wont make a big difference of performance. |
|
If this doesn't affect anything then the only reason to change this code would be that the change makes the code easier to maintain. Looking at the change, I think the new code will be more difficult to maintain as it requires reading all uses to understand the relationship between the fields. Therefore, I'd leave this code alone. |
|
Thinks for your reply, I aggre with what you said. |
Description
Threads which try to 'getAllNode' or 'getActiveConnectorNodes' could be against with thread that are refreshing workers state. This control of race condition seems unnecessary, so removing locks may be improve performance slightly.
Additional context and related issues
No related issues so far.
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.