You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "keith-turner (via GitHub)" <gi...@apache.org> on 2025/01/13 22:44:49 UTC
[PR] Used data size instead of entry size of compaction queue [accumulo]
keith-turner opened a new pull request, #5252:
URL: https://github.com/apache/accumulo/pull/5252
Modified the compaction queue limit to use the data size of the compaction jobs instead of the number of compaction jobs for limiting the number of compaction jobs buffered.
fixes #5186
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Used data size instead of entry size of compaction queue [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner merged PR #5252:
URL: https://github.com/apache/accumulo/pull/5252
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Used data size instead of entry size of compaction queue [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #5252:
URL: https://github.com/apache/accumulo/pull/5252#discussion_r1918720888
##########
server/manager/src/main/java/org/apache/accumulo/manager/compaction/coordinator/CompactionCoordinator.java:
##########
@@ -1121,8 +1121,6 @@ private void cleanUpEmptyCompactorPathInZK() {
final String compactorQueuesPath = this.ctx.getZooKeeperRoot() + Constants.ZCOMPACTORS;
final var zoorw = this.ctx.getZooSession().asReaderWriter();
- final double queueSizeFactor = ctx.getConfiguration()
- .getFraction(Property.MANAGER_COMPACTION_SERVICE_PRIORITY_QUEUE_SIZE_FACTOR);
Review Comment:
Is this property still used?
##########
test/src/main/java/org/apache/accumulo/test/compaction/CompactionPriorityQueueMetricsIT.java:
##########
@@ -202,7 +202,7 @@ public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration conf)
Property.COMPACTION_SERVICE_PREFIX.getKey() + QUEUE1_SERVICE + ".planner.opts.groups",
"[{'group':'" + QUEUE1 + "'}]");
- cfg.setProperty(Property.MANAGER_COMPACTION_SERVICE_PRIORITY_QUEUE_INITIAL_SIZE, "6");
+ cfg.setProperty(Property.MANAGER_COMPACTION_SERVICE_PRIORITY_QUEUE_SIZE, "6");
Review Comment:
I think the previous value was 6 entries, now it's 6 bytes. Is this too small?
##########
core/src/main/java/org/apache/accumulo/core/conf/Property.java:
##########
@@ -465,14 +465,13 @@ public enum Property {
"The number of threads used to seed fate split task, the actual split work is done by fate"
+ " threads.",
"4.0.0"),
-
- MANAGER_COMPACTION_SERVICE_PRIORITY_QUEUE_INITIAL_SIZE(
- "manager.compaction.major.service.queue.initial.size", "10000", PropertyType.COUNT,
- "The initial size of each resource groups compaction job priority queue.", "4.0.0"),
- MANAGER_COMPACTION_SERVICE_PRIORITY_QUEUE_SIZE_FACTOR(
- "manager.compaction.major.service.queue.size.factor", "3.0", PropertyType.FRACTION,
- "The dynamic resizing of the compaction job priority queue is based on"
- + " the number of compactors for the group multiplied by this factor.",
+ // TODO need to add a metric for data size of the queue so this metric can be tuned
Review Comment:
Need to add an issue for this and remove this todo.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Used data size instead of entry size of compaction queue [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #5252:
URL: https://github.com/apache/accumulo/pull/5252#discussion_r1918729432
##########
test/src/main/java/org/apache/accumulo/test/compaction/CompactionPriorityQueueMetricsIT.java:
##########
@@ -202,7 +202,7 @@ public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration conf)
Property.COMPACTION_SERVICE_PREFIX.getKey() + QUEUE1_SERVICE + ".planner.opts.groups",
"[{'group':'" + QUEUE1 + "'}]");
- cfg.setProperty(Property.MANAGER_COMPACTION_SERVICE_PRIORITY_QUEUE_INITIAL_SIZE, "6");
+ cfg.setProperty(Property.MANAGER_COMPACTION_SERVICE_PRIORITY_QUEUE_SIZE, "6");
Review Comment:
Yeah that is too small will change it to something more reasonable and change the test if needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Used data size instead of entry size of compaction queue [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #5252:
URL: https://github.com/apache/accumulo/pull/5252#discussion_r1918729916
##########
server/manager/src/main/java/org/apache/accumulo/manager/compaction/coordinator/CompactionCoordinator.java:
##########
@@ -1121,8 +1121,6 @@ private void cleanUpEmptyCompactorPathInZK() {
final String compactorQueuesPath = this.ctx.getZooKeeperRoot() + Constants.ZCOMPACTORS;
final var zoorw = this.ctx.getZooSession().asReaderWriter();
- final double queueSizeFactor = ctx.getConfiguration()
- .getFraction(Property.MANAGER_COMPACTION_SERVICE_PRIORITY_QUEUE_SIZE_FACTOR);
Review Comment:
no, I forgot to remove that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org