Skip to content

Stops listing all compactors in each compactor#4681

Merged
keith-turner merged 2 commits intoapache:2.1from
keith-turner:accumulo-4666
Jun 18, 2024
Merged

Stops listing all compactors in each compactor#4681
keith-turner merged 2 commits intoapache:2.1from
keith-turner:accumulo-4666

Conversation

@keith-turner
Copy link
Contributor

Each compactor process was listing all other compactors inorder to get a count of compactors. This creates O(N^2) work on zookeeper where N is the number of compactors. This change modifies compactors to get the count from the coordinator via an existing RPC to the coordinator.

Each compactor process was listing all other compactors inorder
to get a count of compactors.  This creates O(N^2) work on zookeeper
where N is the number of compactors.  This change modifies compactors
to get the count from the coordinator via an existing RPC to the
coordinator.
printStartupMsg();
startCompactionCleaner(schedExecutor);
startRunningCleaner(schedExecutor);
// TODO use refresh mechanism to avoid RPCs blocking on getting this count when it expires
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this TODO need to be done before the merge?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that todo is probably an unneeded optimization that is not supported by any data. I added some logging and removed the todo in 51770be. In testing the log message locally noticed it would take a few ms the first count and 0ms for the 2nd count because its reading from zoocache. Thinking the logging can help if the counts ever turn out to be a problem.

protected long getWaitTimeBetweenCompactionChecks() {
protected long getWaitTimeBetweenCompactionChecks(int numCompactors) {
// get the total number of compactors assigned to this queue
int numCompactors = ExternalCompactionUtil.countCompactors(queueName, getContext());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good catch.

@keith-turner keith-turner merged commit bb6143c into apache:2.1 Jun 18, 2024
@keith-turner keith-turner deleted the accumulo-4666 branch June 18, 2024 15:43
@keith-turner keith-turner added this to the 2.1.3 milestone Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Investigate idle compactor load on zookeeper

3 participants