[Improvement-16872][Master] Select a coordinator from masters to wake up task group #16873
Conversation
ruanwenjun
requested review from
Gallardot,
caishunfeng and
SbloodyS
as code owners
ruanwenjun
force-pushed
the
dev_wenjun_addMasterCoordinator
branch
2 times, most recently
from
b9327bb to
680b125
Compare
ruanwenjun
force-pushed
the
dev_wenjun_addMasterCoordinator
branch
from
680b125 to
0b0f97b
Compare
ruanwenjun
force-pushed
the
dev_wenjun_addMasterCoordinator
branch
2 times, most recently
from
ce0e0b9 to
2cf0153
Compare
ruanwenjun
force-pushed
the
dev_wenjun_addMasterCoordinator
branch
6 times, most recently
from
66c1a8b to
dd78545
Compare
ruanwenjun
force-pushed
the
dev_wenjun_addMasterCoordinator
branch
from
dd78545 to
e967b9b
Compare
|
| try { | ||
| if (registry.acquireLock(electionLock)) { | ||
| if (!registry.exists(selectorPath)) { | ||
| registry.put(selectorPath, serverIdentify, true); | ||
| return true; | ||
| } | ||
| return serverIdentify.equals(registry.get(selectorPath)); | ||
| } | ||
| return false; | ||
| } finally { | ||
| registry.releaseLock(electionLock); | ||
| } |
There was a problem hiding this comment.
As the issue for avoid to use lock, but it's actually still using lock, what's the difference?
There was a problem hiding this comment.
The lock is only used when cluster changed, after selected an active server, the lock will be released, so the lock will only hold for a short time. This is only used for election, we use the lock here to make sure all registry plugins can implement the election method in an easy way.
There was a problem hiding this comment.
If we add another coordintor, will all distributed services be executed on this one active master? It seems the Master HA but not some logic coordinator HA, I'm not sure if this adjustment will cause one active master to be busy while others are idle.
public class MasterCoordinator extends AbstractHAServer {
private final ITaskGroupCoordinator taskGroupCoordinator;
}
There was a problem hiding this comment.
Yes, the coordinator master shouldn't do a lot of business work, it would be great if it only deals with coordination work. At our usage case, the coordinator master will only coordinate task group, deal with serial wait this will not affect even though the master is busy, these work will not be affect.
BTW, after fixing a lot of memory leak/thread leak bug, I haven't found a master status change to BUSY in our cluster, currently, there is basically no heavy workload in the masters, most case is db busy.
Purpose of the pull request
close #16872
Brief change log
Verify this pull request
This pull request is code cleanup without any test coverage.
(or)
This pull request is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(or)
Pull Request Notice
Pull Request Notice
If your pull request contain incompatible change, you should also add it to
docs/docs/en/guide/upgrede/incompatible.md