[GitHub] [incubator-druid] leventov commented on a change in pull request #8236: Add TaskResourceCleaner; fix a couple of concurrency bugs in batch tasks
leventov commented on a change in pull request #8236: Add TaskResourceCleaner; fix a couple of concurrency bugs in batch tasks URL: https://github.com/apache/incubator-druid/pull/8236#discussion_r310723222 ## File path: indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java ## @@ -146,10 +146,13 @@ private final RetryPolicyFactory retryPolicyFactory; @JsonIgnore - private List indexTaskSpecs; + private final AppenderatorsManager appenderatorsManager; @JsonIgnore - private AppenderatorsManager appenderatorsManager; + private List indexTaskSpecs; + + @Nullable + private volatile IndexTask currentRunningTaskSpec = null; Review comment: https://github.com/code-review-checklists/java-concurrency#justify-volatile 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8236: Add TaskResourceCleaner; fix a couple of concurrency bugs in batch tasks
leventov commented on a change in pull request #8236: Add TaskResourceCleaner;
fix a couple of concurrency bugs in batch tasks
URL: https://github.com/apache/incubator-druid/pull/8236#discussion_r311040270
##
File path:
indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java
##
@@ -146,10 +146,13 @@
private final RetryPolicyFactory retryPolicyFactory;
@JsonIgnore
- private List indexTaskSpecs;
+ private final AppenderatorsManager appenderatorsManager;
@JsonIgnore
- private AppenderatorsManager appenderatorsManager;
+ private List indexTaskSpecs;
+
+ @Nullable
+ private volatile IndexTask currentRunningTaskSpec = null;
Review comment:
The added Javadoc doesn't answer the question "Why the semantics of volatile
field reads and writes (as defined in the Java Memory Model) are required for
the field?" from the referenced checklist item.
It's actually likely that just adding `volatile` to the field is either not
enough to achieve certain concurrency goal (which is also not clarified in the
Javadoc), or not needed.
"by an HTTP thread when {@link #stopGracefully} is called." - please point
to a specific class and method where does this happen, because it's not obvious.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8236: Add TaskResourceCleaner; fix a couple of concurrency bugs in batch tasks
leventov commented on a change in pull request #8236: Add TaskResourceCleaner;
fix a couple of concurrency bugs in batch tasks
URL: https://github.com/apache/incubator-druid/pull/8236#discussion_r311040270
##
File path:
indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java
##
@@ -146,10 +146,13 @@
private final RetryPolicyFactory retryPolicyFactory;
@JsonIgnore
- private List indexTaskSpecs;
+ private final AppenderatorsManager appenderatorsManager;
@JsonIgnore
- private AppenderatorsManager appenderatorsManager;
+ private List indexTaskSpecs;
+
+ @Nullable
+ private volatile IndexTask currentRunningTaskSpec = null;
Review comment:
The added Javadoc doesn't answer the question "Why the semantics of volatile
field reads and writes (as defined in the Java Memory Model) are required for
the field?" from the referenced checklist item.
It's actually likely that just adding `volatile` to the field is either not
enough to achieve certain concurrency goal (which is also not clarified in the
Javadoc), or not needed.
"by an HTTP thread when `{@link #stopGracefully}` is called." - please point
to a specific class and method where does this happen, because it's not obvious.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8236: Add TaskResourceCleaner; fix a couple of concurrency bugs in batch tasks
leventov commented on a change in pull request #8236: Add TaskResourceCleaner;
fix a couple of concurrency bugs in batch tasks
URL: https://github.com/apache/incubator-druid/pull/8236#discussion_r311232377
##
File path:
indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java
##
@@ -146,10 +146,13 @@
private final RetryPolicyFactory retryPolicyFactory;
@JsonIgnore
- private List indexTaskSpecs;
+ private final AppenderatorsManager appenderatorsManager;
@JsonIgnore
- private AppenderatorsManager appenderatorsManager;
+ private List indexTaskSpecs;
+
+ @Nullable
+ private volatile IndexTask currentRunningTaskSpec = null;
Review comment:
> Do you want me to use the terms defined in the JMM?
Yes, I think the term "happens-before" should usually appear in this type of
explanatory comments.
> So do you think volatile here is necessary or not?
I don't know because I wasn't able to figure out the place you referred to
in "by an HTTP thread when `{@link #stopGracefully}` is called."
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8236: Add TaskResourceCleaner; fix a couple of concurrency bugs in batch tasks
leventov commented on a change in pull request #8236: Add TaskResourceCleaner;
fix a couple of concurrency bugs in batch tasks
URL: https://github.com/apache/incubator-druid/pull/8236#discussion_r311237120
##
File path:
indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java
##
@@ -146,10 +146,13 @@
private final RetryPolicyFactory retryPolicyFactory;
@JsonIgnore
- private List indexTaskSpecs;
+ private final AppenderatorsManager appenderatorsManager;
@JsonIgnore
- private AppenderatorsManager appenderatorsManager;
+ private List indexTaskSpecs;
+
+ @Nullable
+ private volatile IndexTask currentRunningTaskSpec = null;
Review comment:
But first of all, "This variable is updated by the main thread and read by
an HTTP thread when `{@link #stopGracefully}` is called." doesn't explain the
concurrency model in enough detail. Can `{@link #stopGracefully}` be called
fully concurrently at whatever moment in parallel with `runTask()`, or it could
be called only in response to some failure within `runTask()`, just from a
different thread? In the first case, `volatile` is likely needed to ensure
*safe publication* (see JCIP 3.5, or
[here](https://shipilev.net/blog/2014/safe-public-construction/#_safe_publication))
of the `IndexTask` object. In the second case, volatile is almost definitely
redundant, if the thread from which `stopGracefully()` is called "gets
informed" about the failure in `runTask()` in any way that is different from
something like a spin-loop read of a non-volatile field (yes, this is very
obscure and never actually happens in practice).
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8236: Add TaskResourceCleaner; fix a couple of concurrency bugs in batch tasks
leventov commented on a change in pull request #8236: Add TaskResourceCleaner;
fix a couple of concurrency bugs in batch tasks
URL: https://github.com/apache/incubator-druid/pull/8236#discussion_r311608563
##
File path:
indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java
##
@@ -146,10 +146,13 @@
private final RetryPolicyFactory retryPolicyFactory;
@JsonIgnore
- private List indexTaskSpecs;
+ private final AppenderatorsManager appenderatorsManager;
@JsonIgnore
- private AppenderatorsManager appenderatorsManager;
+ private List indexTaskSpecs;
+
+ @Nullable
+ private volatile IndexTask currentRunningTaskSpec = null;
Review comment:
So if `stopGracefully()` may be called at any time concurrently with
`run()`, I think the concurrency property that is worth ensuring is not
starting running the next task if `stopGracefully()` is already stopping the
previous task. To achieve this, `volatile` is not enough. This property could
be achieved the following way:
- `currentRunningTaskSpec` is `AtomicReference`
- The stopping callback:
```java
Object currentRunningTask =
currentRunningTaskSpec.getAndSet(SPECIAL_VALUE_STOPPED);
if (currentRunningTask != null) {
((IndexTask) currentRunningTask).stopGracefull(config);
}
```
- The code in the loop in `runTask()` does something like
```java
Object prevSpec = currentRunningTaskSpec.get();
if (prevTask == SPECIAL_VALUE_STOPPED ||
!currentRunningTaskSpec.compareAndSet(prevSpec, eachSpec)) {
log.info("Stopped concurrently");
...
}
```
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8236: Add TaskResourceCleaner; fix a couple of concurrency bugs in batch tasks
leventov commented on a change in pull request #8236: Add TaskResourceCleaner;
fix a couple of concurrency bugs in batch tasks
URL: https://github.com/apache/incubator-druid/pull/8236#discussion_r311608563
##
File path:
indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java
##
@@ -146,10 +146,13 @@
private final RetryPolicyFactory retryPolicyFactory;
@JsonIgnore
- private List indexTaskSpecs;
+ private final AppenderatorsManager appenderatorsManager;
@JsonIgnore
- private AppenderatorsManager appenderatorsManager;
+ private List indexTaskSpecs;
+
+ @Nullable
+ private volatile IndexTask currentRunningTaskSpec = null;
Review comment:
So if `stopGracefully()` may be called at any time concurrently with
`run()`, I think the concurrency property that is worth ensuring is not
starting running the next task if `stopGracefully()` is already stopping the
previous task. To achieve this, `volatile` is not enough. This property could
be achieved the following way:
- `currentRunningTaskSpec` is `AtomicReference`
- The stopping callback:
```java
Object currentRunningTask =
currentRunningTaskSpec.getAndSet(SPECIAL_VALUE_STOPPED);
if (currentRunningTask != null) {
((IndexTask) currentRunningTask).stopGracefull(config);
}
```
- The code in the loop in `runTask()` does something like
```java
Object prevSpec = currentRunningTaskSpec.get();
if (prevSpec == SPECIAL_VALUE_STOPPED ||
!currentRunningTaskSpec.compareAndSet(prevSpec, eachSpec)) {
log.info("Stopped concurrently");
...
}
```
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8236: Add TaskResourceCleaner; fix a couple of concurrency bugs in batch tasks
leventov commented on a change in pull request #8236: Add TaskResourceCleaner; fix a couple of concurrency bugs in batch tasks URL: https://github.com/apache/incubator-druid/pull/8236#discussion_r311659574 ## File path: indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java ## @@ -146,10 +146,13 @@ private final RetryPolicyFactory retryPolicyFactory; @JsonIgnore - private List indexTaskSpecs; + private final AppenderatorsManager appenderatorsManager; @JsonIgnore - private AppenderatorsManager appenderatorsManager; + private List indexTaskSpecs; + + @Nullable + private volatile IndexTask currentRunningTaskSpec = null; Review comment: > Side note, I don't think this applies here, but in general do you think it's a valid explanation to say: "this field is accessed by multiple threads, and I haven't conclusively proven that volatile is not necessary, so I've included it"? > > Probably a lot of the volatiles in the codebase today are like this. I don't necessarily see a big problem with it. Even though some of them could probably be safely removed, the 'just-in-case' volatiles still help lower the cognitive overhead of documenting and dealing with the codebase. (Otherwise, you'd need to think through these arguments again every time you make a change.) To me, the main problem is that `volatile` is often used to delude oneself or reviewers that some code is concurrently safe without forcing through the threading/concurrent control flow models of the code, the desired properties of the code, and the proof that the code has the desired properties. If "`volatile` = probably worse perf, but no concurrency defects" was the case, that wouldn't be too bad, indeed. Unfortunately, in reality, what we have is "`volatile` = high probability that there are some concurrency defects around the code". 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8236: Add TaskResourceCleaner; fix a couple of concurrency bugs in batch tasks
leventov commented on a change in pull request #8236: Add TaskResourceCleaner; fix a couple of concurrency bugs in batch tasks URL: https://github.com/apache/incubator-druid/pull/8236#discussion_r311659574 ## File path: indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java ## @@ -146,10 +146,13 @@ private final RetryPolicyFactory retryPolicyFactory; @JsonIgnore - private List indexTaskSpecs; + private final AppenderatorsManager appenderatorsManager; @JsonIgnore - private AppenderatorsManager appenderatorsManager; + private List indexTaskSpecs; + + @Nullable + private volatile IndexTask currentRunningTaskSpec = null; Review comment: > Side note, I don't think this applies here, but in general do you think it's a valid explanation to say: "this field is accessed by multiple threads, and I haven't conclusively proven that volatile is not necessary, so I've included it"? > > Probably a lot of the volatiles in the codebase today are like this. I don't necessarily see a big problem with it. Even though some of them could probably be safely removed, the 'just-in-case' volatiles still help lower the cognitive overhead of documenting and dealing with the codebase. (Otherwise, you'd need to think through these arguments again every time you make a change.) To me, the main problem is that `volatile` is often used to delude oneself or reviewers that some code is concurrently safe without forcing through the threading/concurrent control flow models of the code, the desired properties of the code, and the proof that the code has the desired properties. If "`volatile` = probably worse perf, but no concurrency defects" was the case, that wouldn't be too bad, indeed. Unfortunately, in reality, what we have is "`volatile` = high probability that there are some concurrency defects around the code". This is why https://github.com/code-review-checklists/java-concurrency#justify-volatile demands to justify `volatile` in JMM terms rather than in vague phrases. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8236: Add TaskResourceCleaner; fix a couple of concurrency bugs in batch tasks
leventov commented on a change in pull request #8236: Add TaskResourceCleaner; fix a couple of concurrency bugs in batch tasks URL: https://github.com/apache/incubator-druid/pull/8236#discussion_r312194391 ## File path: indexing-service/src/main/java/org/apache/druid/indexing/common/task/Task.java ## @@ -162,15 +162,21 @@ default int getPriority() Review comment: Could you please add an overview section to `Task`'s class-level Javadoc, describing which methods of this interface could be called concurrently with which other methods and which could not? In other words, provide an overview [concurrent access documentation](https://github.com/code-review-checklists/java-concurrency#justify-document) for this interface. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8236: Add TaskResourceCleaner; fix a couple of concurrency bugs in batch tasks
leventov commented on a change in pull request #8236: Add TaskResourceCleaner;
fix a couple of concurrency bugs in batch tasks
URL: https://github.com/apache/incubator-druid/pull/8236#discussion_r312195277
##
File path:
indexing-service/src/main/java/org/apache/druid/indexing/common/task/Task.java
##
@@ -162,15 +162,21 @@ default int getPriority()
/**
* Asks a task to arrange for its "run" method to exit promptly. Tasks that
take too long to stop gracefully will be
- * terminated with extreme prejudice. Note that this method can be called at
any time while {@link #run} is called.
- * Its implementations should handle potential concurreny issues properly.
+ * terminated with extreme prejudice.
+ *
+ * This method can be called at any time while {@link #run} is being called
when the task is killed.
Review comment:
Could you specify what should or should not happen if `stopGracefull()` is
called concurrently (e. g. "before") `run()`? Does it guarantee that the task
won't even start?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8236: Add TaskResourceCleaner; fix a couple of concurrency bugs in batch tasks
leventov commented on a change in pull request #8236: Add TaskResourceCleaner;
fix a couple of concurrency bugs in batch tasks
URL: https://github.com/apache/incubator-druid/pull/8236#discussion_r312556516
##
File path:
indexing-service/src/main/java/org/apache/druid/indexing/common/task/Task.java
##
@@ -164,7 +164,10 @@ default int getPriority()
* Asks a task to arrange for its "run" method to exit promptly. Tasks that
take too long to stop gracefully will be
* terminated with extreme prejudice.
*
- * This method can be called at any time while {@link #run} is being called
when the task is killed.
+ * This method can be called at any time while {@link #run} is being called
when the task is killed. If this task
+ * is not started yet, that is {@link #run} is not called yet, this method
will be never called.
+ * Once this task is started, this method can be called even after {@link
#run} returns. Implementations of this
+ * method may want to avoid unnecessary work if {@link #run} already
returned.
* Depending on the task executor type, one of the two cases below can
happen when the task is killed.
Review comment:
It would be clearer if there was an empty line above this line and no empty
line below this line.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8236: Add TaskResourceCleaner; fix a couple of concurrency bugs in batch tasks
leventov commented on a change in pull request #8236: Add TaskResourceCleaner;
fix a couple of concurrency bugs in batch tasks
URL: https://github.com/apache/incubator-druid/pull/8236#discussion_r312558173
##
File path:
indexing-service/src/main/java/org/apache/druid/indexing/common/task/Task.java
##
@@ -164,7 +164,10 @@ default int getPriority()
* Asks a task to arrange for its "run" method to exit promptly. Tasks that
take too long to stop gracefully will be
* terminated with extreme prejudice.
*
- * This method can be called at any time while {@link #run} is being called
when the task is killed.
+ * This method can be called at any time while {@link #run} is being called
when the task is killed. If this task
+ * is not started yet, that is {@link #run} is not called yet, this method
will be never called.
Review comment:
This text sort of doesn't make sense, because the words like "yet",
"before", "after" are not definable in terms of JMM which doesn't deal with
time. Yet, in practical terms, currently, `stopGracefully()` *can* be called
"before" `run()` in `CompactionTask` on one of the eachSpecs. So this contract
is violated.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8236: Add TaskResourceCleaner; fix a couple of concurrency bugs in batch tasks
leventov commented on a change in pull request #8236: Add TaskResourceCleaner;
fix a couple of concurrency bugs in batch tasks
URL: https://github.com/apache/incubator-druid/pull/8236#discussion_r313083513
##
File path:
indexing-service/src/main/java/org/apache/druid/indexing/common/task/Task.java
##
@@ -164,7 +164,10 @@ default int getPriority()
* Asks a task to arrange for its "run" method to exit promptly. Tasks that
take too long to stop gracefully will be
* terminated with extreme prejudice.
*
- * This method can be called at any time while {@link #run} is being called
when the task is killed.
+ * This method can be called at any time while {@link #run} is being called
when the task is killed. If this task
+ * is not started yet, that is {@link #run} is not called yet, this method
will be never called.
Review comment:
> I think the contract now should say "this method can be called at any time
no matter when run() is executed".
I agree.
> I don't know what it should be to address [your
comment](https://github.com/apache/incubator-druid/pull/8236#discussion_r312195277).
What is your suggestion?
I would just put the phrase "Regardless when `stopGracefully()` is called w.
r. t. `run()`, the implementation must not allow a resource leak or lingering
executions (local or remote)."
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] leventov commented on a change in pull request #8236: Add TaskResourceCleaner; fix a couple of concurrency bugs in batch tasks
leventov commented on a change in pull request #8236: Add TaskResourceCleaner;
fix a couple of concurrency bugs in batch tasks
URL: https://github.com/apache/incubator-druid/pull/8236#discussion_r313113092
##
File path:
indexing-service/src/main/java/org/apache/druid/indexing/common/task/Task.java
##
@@ -164,7 +164,10 @@ default int getPriority()
* Asks a task to arrange for its "run" method to exit promptly. Tasks that
take too long to stop gracefully will be
* terminated with extreme prejudice.
*
- * This method can be called at any time while {@link #run} is being called
when the task is killed.
+ * This method can be called at any time while {@link #run} is being called
when the task is killed. If this task
+ * is not started yet, that is {@link #run} is not called yet, this method
will be never called.
Review comment:
Missed the first sentence: "this method can be called at any time no matter
when run() is executed".
Other than that, I don't have more comments about this PR. I didn't review
it in full but don't block it.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
