Doris-Breakwater commented on issue #66298:
URL: https://github.com/apache/doris/issues/66298#issuecomment-5129983723
## Initial analysis
**Judgment: confirmed bug on `4.1.0-rc03`, with high confidence.** The
reported numbers are consistent with the code reaching the `ColumnString`
uint32 offset ceiling while a non-streaming VARIANT writer accumulates one
output segment. This is an availability issue: the same immutable inputs and
row-count plan will fail again unless the segment layout or writer path
changes. The retry amplification belongs in #66299, but it materially raises
the operational severity of this issue.
The issue currently has no labels or assignee. It should be triaged as a
compaction/VARIANT correctness and availability bug rather than as a
Ceph-specific problem.
### Code findings
1. At `4.1.0-rc03`, `Compaction::get_avg_segment_rows()` uses
`_input_rowsets_data_size`, which is compressed on-disk data, to derive a row
limit from `vertical_compaction_max_segment_size`
(`be/src/storage/compaction/compaction.cpp:319-335`). The units therefore do
not protect an uncompressed in-memory column limit.
2. `ColumnStr<UInt32>::check_chars_length()` rejects a character buffer
above `0xffffffff` (`be/src/core/column/column_string.h:64-78`). The reported
`total_length=4297424560` is 2,457,265 bytes above that limit and is exactly
consistent with the next append crossing it.
3. The important scope detail is that 4 GiB is a limit of one **in-memory
`ColumnString`**, not inherently a limit on all uncompressed string bytes
stored across a segment's pages. The non-streaming
`VariantColumnWriterImpl::append_data()` repeatedly calls
`_column->insert_range_from(...)` and retains that `ColumnVariant` until
segment finalization
(`be/src/storage/segment/variant/variant_column_writer_impl.cpp:1478-1495` at
the tag). That segment-wide materialization turns the row-count sizing mismatch
into this deterministic failure.
4. Vertical compaction fixes output segment row boundaries while writing the
key group. Later value groups must follow each key segment's `row_count()`
(`vertical_beta_rowset_writer.cpp:65-114`). Consequently, a value/VARIANT group
cannot simply open a new segment when it approaches 4 GiB without also changing
the already-established boundaries for every column group. The current
key-group check is also performed before appending the next whole block and
uses `>` rather than `>=`, so the planned row limit can be exceeded by one
input block; this is secondary here but should be covered by tests.
5. The compressed-size estimator is still present on current master.
However, master is not identical to the affected tag in every relevant respect:
- Master propagates `input_rs_readers` into `RowsetWriterContext` and can
use `VariantStreamingCompactionWriter` for nested-group, non-doc-mode VARIANT
columns.
- In `4.1.0-rc03`, `VariantColumnWriterImpl` has the same streaming
predicate, but `Compaction::merge_input_rowsets()` does not populate
`ctx.input_rs_readers`, so that predicate cannot be satisfied through this
compaction path.
- The master streaming path is only a partial mitigation. The fallback
`VariantColumnWriterImpl`, `VariantSubcolumnWriter`, and
`VariantDocCompactWriter` paths still accumulate a segment-wide
`ColumnVariant`. The table properties and full stack are needed to identify
which of these triggered this report.
6. The statement that ordinary plain STRING columns reproduce the same
segment-wide failure is not yet established by the supplied evidence.
`ScalarColumnWriter` writes strings incrementally into pages rather than
retaining a segment-wide `ColumnString`; a plain STRING failure would require a
single merge block/converter column itself to exceed 4 GiB or a different
accumulating operator. Please provide that failure's stack separately if it is
part of the claimed scope.
### Recommended fix direction
The most direct correctness fix is to extend chunked/streaming compaction
writing to all affected VARIANT modes and extracted/doc subcolumn writer paths,
so an output segment may contain more than 4 GiB of uncompressed string payload
across encoded pages without constructing one `ColumnString` above the uint32
limit. The streaming implementation must also split an incoming chunk before
insertion if that chunk alone would cross the limit.
Changing `get_avg_segment_rows()` to use footer `raw_data_bytes` or another
uncompressed estimate is useful as a memory/segment-sizing guard, but it is not
a proof of safety: averages do not cover skew, some VARIANT footer paths do not
currently provide usable raw-byte statistics, and the key-group-first layout
cannot discover a later value column's exact safe boundary dynamically.
Compressed-byte sizing may remain as the on-disk target, but the in-memory
writer invariant needs an independent enforcement mechanism.
If maintainers instead choose dynamic segment cutting, the safe row
boundaries must be determined before or while producing the key group and then
replayed for all value groups. Cutting only inside the later VARIANT group
would violate vertical segment row alignment.
### Information still needed
- Full symbolized stack trace for the quoted E-3113, including the frames
between `ColumnStr::insert_range_from` and the compaction entry point.
- `SHOW CREATE TABLE`, especially VARIANT properties controlling doc mode,
nested groups, typed/extracted paths, and maximum subcolumns.
- Relevant BE settings: `enable_vertical_compaction`,
`enable_vertical_compact_variant_subcolumns`,
`vertical_compaction_max_segment_size`,
`vertical_compaction_num_columns_per_group`, and `compaction_batch_size`.
- The failing compaction's input rowset count, total rows, compressed data
size, per-input segment row counts, and the logged `max_rows_per_segment`.
- For the “plain STRING also works” claim, a separate DDL, minimal
reproduction, error line, and stack trace.
### Suggested next steps
1. Reproduce with a test-only low string-buffer threshold so the regression
does not require allocating 4 GiB, covering regular VARIANT, doc mode,
extracted subcolumns, and nested-group streaming.
2. Verify that compaction succeeds and row counts/checksums match while the
writer never retains more than the test threshold in one in-memory string
column.
3. Add a skewed-data case and a boundary case where the next merge block
would cross the limit; also assert that all vertical column groups use
identical segment row boundaries.
4. Backport the writer fix to the affected 4.1 branch independently of the
scheduler/backoff work in #66299.
Breakwater-GitHub-Analysis-Slot: slot_9859ad160d24
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
