ihuzenko commented on a change in pull request #1929: DRILL-6832: Remove the
old "unmanaged" external sort
URL: https://github.com/apache/drill/pull/1929#discussion_r360368665
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/BatchGroup.java
##########
@@ -18,140 +18,300 @@
package org.apache.drill.exec.physical.impl.xsort;
import java.io.IOException;
+import java.io.InputStream;
+import java.util.Collection;
import java.util.Iterator;
import java.util.concurrent.TimeUnit;
+import org.apache.drill.common.exceptions.UserException;
import org.apache.drill.common.expression.SchemaPath;
-import org.apache.drill.exec.cache.VectorAccessibleSerializable;
+import org.apache.drill.exec.cache.VectorSerializer;
+import org.apache.drill.exec.cache.VectorSerializer.Writer;
import org.apache.drill.exec.memory.BufferAllocator;
-import org.apache.drill.exec.ops.OperatorContext;
+import org.apache.drill.exec.physical.impl.spill.SpillSet;
import org.apache.drill.exec.record.BatchSchema;
import org.apache.drill.exec.record.SchemaUtil;
import org.apache.drill.exec.record.TransferPair;
import org.apache.drill.exec.record.TypedFieldId;
import org.apache.drill.exec.record.VectorAccessible;
import org.apache.drill.exec.record.VectorContainer;
import org.apache.drill.exec.record.VectorWrapper;
-import org.apache.drill.exec.record.WritableBatch;
import org.apache.drill.exec.record.selection.SelectionVector2;
import org.apache.drill.exec.record.selection.SelectionVector4;
-import org.apache.hadoop.fs.FSDataInputStream;
-import org.apache.hadoop.fs.FSDataOutputStream;
-import org.apache.hadoop.fs.FileSystem;
-import org.apache.hadoop.fs.Path;
import org.apache.drill.shaded.guava.com.google.common.base.Stopwatch;
-public class BatchGroup implements VectorAccessible, AutoCloseable {
+/**
+ * Represents a group of batches spilled to disk.
+ * <p>
+ * The batches are defined by a schema which can change over time. When the
schema changes,
+ * all existing and new batches are coerced into the new schema. Provides a
+ * uniform way to iterate over records for one or more batches whether
+ * the batches are in memory or on disk.
+ * <p>
+ * The <code>BatchGroup</code> operates in two modes as given by the two
+ * subclasses:
+ * <ul>
+ * <li>Input mode (@link InputBatchGroup): Used to buffer in-memory batches
+ * prior to spilling.</li>
+ * <li>Spill mode (@link SpilledBatchGroup): Holds a "memento" to a set
+ * of batches written to disk. Acts as both a reader and writer for
+ * those batches.</li>
+ */
+
+public abstract class BatchGroup implements VectorAccessible, AutoCloseable {
static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(BatchGroup.class);
- private VectorContainer currentContainer;
- private SelectionVector2 sv2;
- private int pointer = 0;
- private FSDataInputStream inputStream;
- private FSDataOutputStream outputStream;
- private Path path;
- private FileSystem fs;
- private BufferAllocator allocator;
- private int spilledBatches = 0;
- private OperatorContext context;
- private BatchSchema schema;
-
- public BatchGroup(VectorContainer container, SelectionVector2 sv2,
OperatorContext context) {
- this.sv2 = sv2;
- this.currentContainer = container;
- this.context = context;
- }
+ /**
+ * The input batch group gathers batches buffered in memory before
+ * spilling. The structure of the data is:
+ * <ul>
+ * <li>Contains a single batch received from the upstream (input)
+ * operator.</li>
+ * <li>Associated selection vector that provides a sorted
+ * indirection to the values in the batch.</li>
+ * </ul>
+ */
- public BatchGroup(VectorContainer container, FileSystem fs, String path,
OperatorContext context) {
- currentContainer = container;
- this.fs = fs;
- this.path = new Path(path);
- this.allocator = context.getAllocator();
- this.context = context;
- }
+ public static class InputBatch extends BatchGroup {
Review comment:
I really appreciate that you split the responsibilities of ```BatchGroup```
class and created ```InputBatch``` & ```SpilledRun```. Could you please
extract them into separate source files?
----------------------------------------------------------------
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
