Skip to content

Consider waiting and pending compaction tasks as well as running tasks in DruidCoordinatorSegmentCompactor#5704

Merged
b-slim merged 3 commits intoapache:masterfrom
jihoonson:coordinator-compactor-task-num
May 9, 2018
Merged

Consider waiting and pending compaction tasks as well as running tasks in DruidCoordinatorSegmentCompactor#5704
b-slim merged 3 commits intoapache:masterfrom
jihoonson:coordinator-compactor-task-num

Conversation

@jihoonson
Copy link
Copy Markdown
Contributor

@jihoonson jihoonson commented Apr 25, 2018

DruidCoordinatorSegmentCompactor currently computes the available number of task slots for compaction based on the number of running compaction tasks, i.e., total number of task slots - number of running compaction tasks. This PR is to consider pending and waiting compaction tasks as well to compute more accurate available compaction task slots.

This change is Reviewable

@b-slim
Copy link
Copy Markdown
Contributor

b-slim commented May 3, 2018

@jihoonson can you please provide more context like what this is improving? Thanks

@jihoonson jihoonson added Bug and removed Improvement labels May 3, 2018
@jihoonson
Copy link
Copy Markdown
Contributor Author

@b-slim updated.

// compaction is enabled and numRunningCompactTasks is 0.
Math.max(1, compactionTaskCapacity);
LOG.info("Running tasks [%d/%d]", numRunningCompactTasks, compactionTaskCapacity);
LOG.info("Running tasks [%d/%d]", numNonCompleteCompactionTasks, compactionTaskCapacity);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we make this more readable something like Total running and waiting tasks [], Maximum compaction queue capacity []. Thanks

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point. Fixed.

@b-slim
Copy link
Copy Markdown
Contributor

b-slim commented May 3, 2018

👍 left minor comment.

@b-slim b-slim added this to the 0.13.0 milestone May 4, 2018
@jihoonson
Copy link
Copy Markdown
Contributor Author

@b-slim do you have more comments?

@b-slim
Copy link
Copy Markdown
Contributor

b-slim commented May 8, 2018

nop was waiting if someone else want to review it, i will merge it by end of day if no one look at this.

@jihoonson
Copy link
Copy Markdown
Contributor Author

@b-slim thanks.

@b-slim b-slim merged commit c7a5939 into apache:master May 9, 2018
jon-wei pushed a commit that referenced this pull request May 10, 2018
* Revert "Consider waiting and pending compaction tasks as well as running tasks in DruidCoordinatorSegmentCompactor (#5704)"

This reverts commit c7a5939.

* Revert "Fix metrics for inserting segments (#5749)"

This reverts commit c9d6451.

* Revert "Typo fix in historical doc (#5753)"

This reverts commit aa23fe6.

* Revert "Use a bimap for reverse lookups on injective maps (#5681)"

This reverts commit e1277d3.
jihoonson added a commit to implydata/druid-public that referenced this pull request Jun 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants