Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/1058#discussion_r154476440 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorSerializer.java --- @@ -40,20 +52,18 @@ */ public static class Writer { + static final MetricRegistry metrics = DrillMetrics.getRegistry(); + static final String WRITER_TIMER = MetricRegistry.name(VectorAccessibleSerializable.class, "writerTime"); - private final OutputStream stream; - private final BufferAllocator allocator; - private boolean retain; + private final SpillSet spillSet; + private final WritableByteChannel channel; + private final OutputStream output; private long timeNs; - public Writer(BufferAllocator allocator, OutputStream stream) { - this.allocator = allocator; - this.stream = stream; - } - - public Writer retain() { - retain = true; - return this; + private Writer(SpillSet spillSet, String path) throws IOException { --- End diff -- Before this class was independent of the `SpillSet`. Now, it can be used only with that class. Not sure if this is an improvement. The only thing it seems we need from `SpillSet` is to update the write byte count. Perhaps define an interface here with just the needed methods. Then, let `SpillSet` implement that interface. That gets what you need without introducing extra dependencies.
---