gianm commented on code in PR #19223:
URL: https://github.com/apache/druid/pull/19223#discussion_r3049315363
##########
indexing-service/src/main/java/org/apache/druid/indexing/compact/OverlordCompactionScheduler.java:
##########
@@ -264,6 +266,8 @@ public CompactionConfigValidationResult
validateCompactionConfig(DataSourceCompa
{
if (compactionConfig == null) {
return CompactionConfigValidationResult.failure("Cannot be null");
+ } else if (compactionConfig instanceof CascadingReindexingTemplate) {
Review Comment:
It worries me to have validation dependent on dynamic configuration in the
constructor of `CompactionSupervisorSpec`. What happens if dynamic
configuration changes in such a way that a previously-valid spec is now
invalid? Will it now fail to show up in the supervisor list API? It will be
tough for the user to fix it if they can't see it.
I believe the ideal approach would be to validate on submission (in
`SupervisorResource#specPost`) and at runtime (in `resetCompactionJobQueue`)
but not on construction.
--
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]
