voonhous commented on code in PR #19181:
URL: https://github.com/apache/hudi/pull/19181#discussion_r3542673414
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/io/storage/LanceRecordIterator.java:
##########
@@ -32,39 +32,54 @@
import org.apache.spark.sql.types.StructType;
import org.apache.spark.sql.vectorized.ColumnVector;
import org.apache.spark.sql.vectorized.ColumnarBatch;
+import org.lance.file.FileReadOptions;
import org.lance.file.LanceFileReader;
import org.lance.spark.vectorized.LanceArrowColumnVector;
+import org.lance.util.Range;
import java.io.IOException;
+import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
/**
- * Iterator for reading Lance files and converting Arrow batches to Spark
{@link UnsafeRow}s.
- * Used by both Hudi's internal Lance reader and Spark datasource integration.
+ * Iterator over a Lance file that converts Arrow batches to Spark {@link
UnsafeRow}s, owning the
+ * {@link BufferAllocator}, {@link LanceFileReader}, {@link ArrowReader}(s)
and current
+ * {@link ColumnarBatch}. Used by both Hudi's internal Lance reader and the
Spark datasource. An
+ * optional {@link BlobDescriptorTransform} rewrites BLOB columns for
DESCRIPTOR-mode reads.
*
- * <p>The iterator manages the lifecycle of:
- * <ul>
- * <li>BufferAllocator - Arrow memory management</li>
- * <li>LanceFileReader - Lance file handle</li>
- * <li>ArrowReader - Arrow batch reader</li>
- * <li>ColumnarBatch - Current batch being iterated</li>
- * </ul>
- *
- * <p>An optional {@link BlobDescriptorTransform} can be composed in to
rewrite BLOB columns
- * in DESCRIPTOR mode.
+ * <p>BLOB chunked reading: lance-core 4.0.0 aborts the JVM in its Arrow
C-stream export whenever a
+ * single {@code readAll} stream crosses Lance's internal BLOB page boundary
(512 rows), and the
+ * requested {@code batchSize} does not change that. So BLOB reads issue one
{@code readAll} per
+ * row-range chunk of {@link #BLOB_READ_CHUNK_ROWS} rows; non-BLOB reads use
one streamed reader.
*/
public final class LanceRecordIterator implements ClosableIterator<UnsafeRow> {
+
+ /** Rows per {@code readAll} for BLOB reads. Must not exceed Lance's 512-row
BLOB page size. */
+ public static final int BLOB_READ_CHUNK_ROWS = 512;
Review Comment:
Two silent couplings that would each bring the JVM abort back with no test
signal - non-blocking, but "silent" is what made this one expensive, so cheap
loud guards feel worth it:
**1. This constant hard-codes lance-core 4.0.0's internal BLOB page size.**
A lance bump that changes it re-introduces straddling chunks silently - nothing
fails unless a >512-row fixture happens to catch it. Worth deriving/asserting
it against lance, or at least pinning a comment to the lance version so the
next upgrader revalidates.
**2. Blob detection (`isBlobField()` over top-level fields) is
top-level-only.** Correct today since the writer only supports top-level BLOB
columns, but the schema layer already models nested blobs (`nestedNullableBlob`
/ `mapWithNestedBlob` / `arrayWithNestedBlob` in `TestHoodieSchema`, plus the
nested-record case in `TestHoodieSchemaCompatibility`). If the writer ever
gains nested-blob support, a schema like:
```sql
CREATE TABLE docs (
id INT,
document STRUCT<source_url: STRING, content_type: STRING, raw_bytes: BLOB>
);
```
silently skips chunking on read and aborts again once a base file crosses
512 rows: the top-level field `document` is a STRUCT, not a blob /
array-of-blob / map-of-blob, so `isBlobField()` returns false and the
non-chunked path is taken. `HoodieSchema.containsBlobType()` already does the
recursive check as a drop-in, or add an assertion that a projected blob isn't
nested so a future writer change fails loud instead of aborting the JVM.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
