You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "kfaraz (via GitHub)" <gi...@apache.org> on 2023/12/18 03:28:04 UTC
[PR] Improve polling in segment allocation queue (druid)
kfaraz opened a new pull request, #15590:
URL: https://github.com/apache/druid/pull/15590
### Description
When `batchAllocationWaitTime` is set to `0`, the segment allocation queue is polled continuously even when it is empty. This would take up cpu cycles unnecessarily.
### Changes
- Poll segment allocation queue only when non-empty
- When a new request is received, schedule a poll
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Improve polling in segment allocation queue (druid)
Posted by "kfaraz (via GitHub)" <gi...@apache.org>.
kfaraz commented on PR #15590:
URL: https://github.com/apache/druid/pull/15590#issuecomment-1863804076
Thanks for the suggestion, @AmatyaAvadhanula . To help me understand better, would a more precise statement instead of `Requests with lineage check cannot be batched.` be that `Requests with lineage check are only sent to the overlord one by one as each subsequent request needs the previous segment id, so there is no scope of batching them.`?
I ask this because the `SegmentAllocationQueue` code itself does not distinguish between lineage check or no lineage check.
Either way, my plan is to eventually get rid of `batchAllocationWaitTime` altogether, so that no request needs to wait unless some other batch is already being processed. That would benefit all types of requests. The change in this PR is just to stabilize the code so that we can try out `batchAllocationWaitTime = 0` before removing the config or updating its default values.
Please let me know what you think.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Improve polling in segment allocation queue (druid)
Posted by "kfaraz (via GitHub)" <gi...@apache.org>.
kfaraz commented on code in PR #15590:
URL: https://github.com/apache/druid/pull/15590#discussion_r1429464111
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/actions/SegmentAllocationQueue.java:
##########
@@ -196,6 +204,7 @@ public Future<SegmentIdWithShardSpec> add(SegmentAllocateRequest request)
}
});
+ scheduleQueuePoll(maxWaitTimeMillis);
Review Comment:
Yeah, makes sense. Let me fix it up.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Improve polling in segment allocation queue (druid)
Posted by "AmatyaAvadhanula (via GitHub)" <gi...@apache.org>.
AmatyaAvadhanula merged PR #15590:
URL: https://github.com/apache/druid/pull/15590
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Improve polling in segment allocation queue (druid)
Posted by "AmatyaAvadhanula (via GitHub)" <gi...@apache.org>.
AmatyaAvadhanula commented on PR #15590:
URL: https://github.com/apache/druid/pull/15590#issuecomment-1865473204
Yes, it would be a more precise statement.
My suggestion was that the code could be changed to distinguish such requests and process them without waiting for `batchAllocationWaitTime` to elapse.
Thank you for the explanation, @kfaraz. I understand the intent of this change better now.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Improve polling in segment allocation queue (druid)
Posted by "AmatyaAvadhanula (via GitHub)" <gi...@apache.org>.
AmatyaAvadhanula commented on code in PR #15590:
URL: https://github.com/apache/druid/pull/15590#discussion_r1429453811
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/actions/SegmentAllocationQueue.java:
##########
@@ -196,6 +204,7 @@ public Future<SegmentIdWithShardSpec> add(SegmentAllocateRequest request)
}
});
+ scheduleQueuePoll(maxWaitTimeMillis);
Review Comment:
Should this be part of the method `addBatchToQueue` itself?
1. With the current code, I think it's possible that requeued batches may not be processed if new requests don't come in.
2. I also don't think we need to schedule a poll for every new request, but just for the first one for a batch.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Improve polling in segment allocation queue (druid)
Posted by "kfaraz (via GitHub)" <gi...@apache.org>.
kfaraz commented on code in PR #15590:
URL: https://github.com/apache/druid/pull/15590#discussion_r1429465364
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/actions/SegmentAllocationQueue.java:
##########
@@ -196,6 +204,7 @@ public Future<SegmentIdWithShardSpec> add(SegmentAllocateRequest request)
}
});
+ scheduleQueuePoll(maxWaitTimeMillis);
Review Comment:
Fixed.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Improve polling in segment allocation queue (druid)
Posted by "AmatyaAvadhanula (via GitHub)" <gi...@apache.org>.
AmatyaAvadhanula commented on PR #15590:
URL: https://github.com/apache/druid/pull/15590#issuecomment-1863077172
Requests with lineage check cannot be batched. Would it help to not wait before polling for such requests?
Tasks such as MSQ that use lineage check may benefit from it even if the batchAllocationWaitTime is set to a high value.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Improve polling in segment allocation queue (druid)
Posted by "kfaraz (via GitHub)" <gi...@apache.org>.
kfaraz commented on PR #15590:
URL: https://github.com/apache/druid/pull/15590#issuecomment-1862051809
Update: Added a few more changes to handle race conditions better and make the code simpler.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Improve polling in segment allocation queue (druid)
Posted by "AmatyaAvadhanula (via GitHub)" <gi...@apache.org>.
AmatyaAvadhanula commented on PR #15590:
URL: https://github.com/apache/druid/pull/15590#issuecomment-1876923326
LGTM @kfaraz!
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org