discivigour commented on code in PR #9928:
URL: https://github.com/apache/paimon/pull/9928#discussion_r4043940342
##########
paimon-format/src/main/java/org/apache/paimon/format/avro/AvroFileFormat.java:
##########
@@ -67,13 +68,15 @@ public class AvroFileFormat extends FileFormat {
private final Options options;
private final int zstdLevel;
@Nullable private final MemorySize blockSize;
+ private final boolean useBufferedEncoder;
public AvroFileFormat(FormatContext context) {
super(IDENTIFIER);
this.options = getIdentifierPrefixOptions(context.options());
this.zstdLevel = context.zstdLevel();
this.blockSize = context.blockSize();
+ this.useBufferedEncoder = !context.isManifest();
Review Comment:
Manifest entries can currently be written through three paths:
- Normal record serialization via append().
- Pre-encoded record reuse via appendEncoded().
- Raw block reuse via appendAllFrom().
These paths can be mixed within the same writer. I kept the direct encoder
for manifests for two reasons:
1. **The pre-encoded record path incurs extra overhead, while raw block
copying does not directly benefit.** In Avro 1.11.4, the buffered encoder turns
the array-backed `ByteBuffer` passed to `appendEncoded()` into a read-only
view, resulting in a temporary array allocation and a full-record copy. Raw
block copying bypasses per-record encoding, so buffering provides no direct
benefit there.
2. The normal record-writing path may benefit, but we expect the gain to be
more limited. In our benchmark with 20 MAP<STRING, ARRAY<INT>> fields, 20 keys
per map, and 300 integers per key—120,000 small integer elements per row—the
observed improvement was only about 10%. For a 500-column table, manifest
entries have far fewer individually encoded small values: min/max statistics
are written as byte blobs, with per-column scalar encoding mainly coming from
arrays such as nullCounts. This suggests less opportunity for small-write
batching. And *enabling it without penalizing the pre-encoded path requires
additional implementation code and testing.** It would need mixed-write,
rollback, block-boundary, and performance coverage.
For this PR, keeping the existing direct encoder for manifests avoids
introducing that overhead and keeps the change scoped to data-file writes.
--
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]
