QiuMM commented on a change in pull request #6724: Fix issue that tasks failed
because of no sink for identifier
URL: https://github.com/apache/incubator-druid/pull/6724#discussion_r243741074
##########
File path:
server/src/main/java/org/apache/druid/segment/realtime/appenderator/AppenderatorImpl.java
##########
@@ -485,8 +486,12 @@ public void clear() throws InterruptedException
final List<Pair<FireHydrant, SegmentIdentifier>> indexesToPersist = new
ArrayList<>();
int numPersistedRows = 0;
long bytesPersisted = 0L;
- for (SegmentIdentifier identifier : sinks.keySet()) {
- final Sink sink = sinks.get(identifier);
+ Iterator<Map.Entry<SegmentIdentifier, Sink>> iterator =
sinks.entrySet().iterator();
+
+ while (iterator.hasNext()) {
Review comment:
> So I'm wondering if it would be more correct to have this "find sinks to
persist" code run in one callable together with the actual persist work in the
persistExecutor as well.
you are right, `persistExecutor` using a single thread, I think it will work.
> There is also a code block in push that has a similar pattern that runs
outside of the pushExecutor (abandonSegment uses pushBarrier()), I wonder if
there are similar problems there as well:
I have never observed any exceptions caused by this. And I think there is no
need to worry about it because the program will wait for any outstanding pushes
to finish, then abandon the segment inside the persist thread:
```java
.....
// Wait for any outstanding pushes to finish, then abandon the segment
inside the persist thread.
return Futures.transform(
pushBarrier(),
new Function<Object, Object>()
{
@Nullable
@Override
......
```java
How do you think?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
