Skip to content

Commit

Permalink
Sequences refactorings and removed unused code (part of apache#3798) (a…
Browse files Browse the repository at this point in the history
…pache#3693)

* Removing unused code from io.druid.java.util.common.guava package; fix apache#3563 (more consistent and paranoiac resource handing in Sequences subsystem); Add Sequences.wrap() for DRY in MetricsEmittingQueryRunner, CPUTimeMetricQueryRunner and SpecificSegmentQueryRunner; Catch MissingSegmentsException in SpecificSegmentQueryRunner's yielder.next() method (follow up on apache#3617)

* Make Sequences.withEffect() execute the effect if the wrapped sequence throws exception from close()

* Fix strange code in MetricsEmittingQueryRunner

* Add comment on why YieldingSequenceBase is used in Sequences.withEffect()

* Use Closer in OrderedMergeSequence and MergeSequence to close multiple yielders
  • Loading branch information
leventov authored and fjy committed Jan 20, 2017
1 parent 9cc3015 commit af93a8d
Show file tree
Hide file tree
Showing 34 changed files with 674 additions and 1,053 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.google.common.base.Throwables;
import com.google.common.collect.Ordering;

import com.google.common.io.Closer;
import io.druid.java.util.common.guava.Accumulator;
import io.druid.java.util.common.guava.CloseQuietly;
import io.druid.java.util.common.guava.Sequence;
Expand Down Expand Up @@ -207,9 +208,11 @@ public boolean isDone()
@Override
public void close() throws IOException
{
Closer closer = Closer.create();
while(!pQueue.isEmpty()) {
pQueue.remove().close();
closer.register(pQueue.remove());
}
closer.close();
}
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,12 @@
import com.google.common.collect.Iterators;
import com.google.common.collect.Lists;
import com.google.common.collect.Ordering;

import io.druid.java.util.common.Pair;
import io.druid.java.util.common.guava.ResourceClosingSequence;
import io.druid.java.util.common.guava.Sequence;
import io.druid.java.util.common.guava.Sequences;
import io.druid.java.util.common.guava.Yielder;
import io.druid.java.util.common.guava.YieldingAccumulator;
import io.druid.java.util.common.guava.nary.BinaryFn;

import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -232,7 +229,7 @@ public void close() throws IOException

Sequence<Pair<Integer, Integer>> seq = Sequences.limit(
new CombiningSequence<>(
new ResourceClosingSequence<>(Sequences.simple(pairs), closeable),
Sequences.withBaggage(Sequences.simple(pairs), closeable),
Ordering.natural().onResultOf(Pair.<Integer, Integer>lhsFn()),
new BinaryFn<Pair<Integer, Integer>, Pair<Integer, Integer>, Pair<Integer, Integer>>()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@

package io.druid.java.util.common.guava;

import com.google.common.base.Throwables;
import io.druid.java.util.common.logger.Logger;

import java.io.Closeable;
import java.io.IOException;
import java.util.Iterator;
Expand All @@ -30,30 +27,9 @@
*/
public class BaseSequence<T, IterType extends Iterator<T>> implements Sequence<T>
{
private static final Logger log = new Logger(BaseSequence.class);

private final IteratorMaker<T, IterType> maker;

public static <T> Sequence<T> simple(final Iterable<T> iterable)
{
return new BaseSequence<>(
new BaseSequence.IteratorMaker<T, Iterator<T>>()
{
@Override
public Iterator<T> make()
{
return iterable.iterator();
}

@Override
public void cleanup(Iterator<T> iterFromMake)
{

}
}
);
}

public BaseSequence(
IteratorMaker<T, IterType> maker
)
Expand All @@ -69,11 +45,18 @@ public <OutType> OutType accumulate(OutType initValue, final Accumulator<OutType
while (iterator.hasNext()) {
initValue = fn.accumulate(initValue, iterator.next());
}
return initValue;
}
finally {
maker.cleanup(iterator);
catch (Throwable t) {
try {
maker.cleanup(iterator);
}
catch (Exception e) {
t.addSuppressed(e);
}
throw t;
}
maker.cleanup(iterator);
return initValue;
}

@Override
Expand All @@ -84,16 +67,14 @@ public <OutType> Yielder<OutType> toYielder(OutType initValue, YieldingAccumulat
try {
return makeYielder(initValue, accumulator, iterator);
}
catch (Exception e) {
// We caught an Exception instead of returning a really, real, live, real boy, errr, iterator
// So we better try to close our stuff, 'cause the exception is what is making it out of here.
catch (Throwable t) {
try {
maker.cleanup(iterator);
}
catch (RuntimeException e1) {
log.error(e1, "Exception thrown when closing maker. Logging and ignoring.");
catch (Exception e) {
t.addSuppressed(e);
}
throw Throwables.propagate(e);
throw t;
}
}

Expand Down Expand Up @@ -138,16 +119,14 @@ public Yielder<OutType> next(OutType initValue)
try {
return makeYielder(initValue, accumulator, iter);
}
catch (Exception e) {
// We caught an Exception instead of returning a really, real, live, real boy, errr, iterator
// So we better try to close our stuff, 'cause the exception is what is making it out of here.
catch (Throwable t) {
try {
maker.cleanup(iter);
}
catch (RuntimeException e1) {
log.error(e1, "Exception thrown when closing maker. Logging and ignoring.");
catch (Exception e) {
t.addSuppressed(e);
}
throw Throwables.propagate(e);
throw t;
}
}

Expand All @@ -165,10 +144,10 @@ public void close() throws IOException
};
}

public static interface IteratorMaker<T, IterType extends Iterator<T>>
public interface IteratorMaker<T, IterType extends Iterator<T>>
{
public IterType make();
IterType make();

public void cleanup(IterType iterFromMake);
void cleanup(IterType iterFromMake);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import com.google.common.base.Throwables;

import java.io.Closeable;
import java.io.IOException;

/**
Expand Down Expand Up @@ -73,11 +74,14 @@ public Sequence<T> accumulate(Sequence<T> accumulated, Sequence<T> in)
try {
return makeYielder(yielderYielder, initValue, accumulator);
}
catch (RuntimeException e) {
// We caught a RuntimeException instead of returning a really, real, live, real boy, errr, iterator
// So we better try to close our stuff, 'cause the exception is what is making it out of here.
CloseQuietly.close(yielderYielder);
throw e;
catch (Throwable t) {
try {
yielderYielder.close();
}
catch (Exception e) {
t.addSuppressed(e);
}
throw t;
}
}

Expand All @@ -87,10 +91,6 @@ public <OutType> Yielder<OutType> makeYielder(
YieldingAccumulator<OutType, T> accumulator
)
{
if (yielderYielder.isDone()) {
return Yielders.done(initValue, yielderYielder);
}

while (!yielderYielder.isDone()) {
Yielder<OutType> yielder = yielderYielder.get().toYielder(initValue, accumulator);
if (accumulator.yielded()) {
Expand Down Expand Up @@ -152,8 +152,9 @@ public boolean isDone()
@Override
public void close() throws IOException
{
yielder.close();
yielderYielder.close();
try (Closeable toClose = yielderYielder) {
yielder.close();
}
}
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,28 @@ public boolean isDone()
@Override
public void close() throws IOException
{
if (isDone()) {
executor.execute(runnable);
boolean done = isDone();
Throwable thrown = null;
try {
baseYielder.close();
}
catch (Throwable t) {
thrown = t;
throw t;
}
finally {
if (done) {
if (thrown != null) {
try {
executor.execute(runnable);
}
catch (Throwable t) {
thrown.addSuppressed(t);
}
} else {
executor.execute(runnable);
}
}
}
baseYielder.close();
}
}

This file was deleted.

Loading

0 comments on commit af93a8d

Please sign in to comment.