LakshSingla commented on code in PR #15243:
URL: https://github.com/apache/druid/pull/15243#discussion_r1381193342
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java:
##########
@@ -1423,14 +1424,18 @@ private void publishAllSegments(final Set<DataSegment>
segments) throws IOExcept
intervalsToDrop,
destination.getReplaceTimeChunks(),
task.getDataSource(),
- destination.getSegmentGranularity()
+ destination.getSegmentGranularity(),
+ Limits.MAX_PARTITION_BUCKETS
);
segmentsWithTombstones.addAll(tombstones);
numTombstones = tombstones.size();
}
catch (IllegalStateException e) {
throw new MSQException(e, InsertLockPreemptedFault.instance());
}
+ catch (IllegalArgumentException e) {
+ throw new MSQException(e, new
TooManyBucketsFault(Limits.MAX_PARTITION_BUCKETS));
+ }
Review Comment:
We can pull out the exception above if that helps
`IAE` to me is more like entering a negative integer instead of a positive
one, or a different Java type when the conditions state a certain type to be
expected. Or perhaps there are "XOR args" (enter exactly one) and the user
passes both.
The logic inside the helper operates before comparing it with the
`maxBuckets` parameter, which doesn't seem like an instance of "invalid
argument".
As per `InsertLockPreempted,` it might be insufficiency of the code at the
time when it was written, but `ISE` seems more forgiving than `IAE` (as to what
all can be `ISE`). Also, that logic in itself is also pretty fragile in the
first place, if the logic in the calling code switches to `DruidException`,
then it would break.
At worst, IAE generated by the helper can have some special text inside, and
we do a string compare before throwing this exception, however since we already
have a special exception created, that seems like a valid path to follow
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
