924060929 commented on PR #66717:
URL: https://github.com/apache/doris/pull/66717#issuecomment-5355800648
Architecture follow-up after re-evaluating the estimator design and the
earlier AI review findings on current head `f6812356`.
My approval remains: I do not see a remaining P1 correctness blocker on this
head. This comment is about converging the design before merge, especially the
production sizing model.
## The PR has absorbed useful findings, but the estimator objective drifted
The original operational goal is to keep long-lived external-metadata cache
retention approximately bounded and let the JVM reclaim unused values under
pressure. For that goal, the important architecture is now present:
- `softValues`, without strong `V` references in reservation/refresh
ownership;
- entry/catalog/global admission budgets;
- admission before publication and generation-fenced release;
- O(1) Caffeine weighing/removal accounting using the publication weight;
- count limits plus matching cleanup on replacement, eviction, collection,
invalidation and close.
However, `max-weight` was then treated as a near-exact retained-byte
promise. The calibration tests require no under-estimation and at most 10%
over-estimation against JOL. That changed the production estimator from a
coarse cache governor into a hand-written model of JVM and third-party SDK
object graphs.
The review loop reinforced this drift. Individual P2 findings such as
missing partition width, alias cardinality, Paimon store growth, default
literals, shared strings, merged ranges and empty-table key width were locally
valid under an exact-byte objective. Fixing every one at object-layout
granularity led to more class fingerprints, lazy-map models, boxed-ID
accounting, HashMap capacity calculations and JOL fixtures. Each local fix
improved one oracle, but the accumulated result is substantially more complex
and SDK-sensitive than the cache-governance problem requires.
## Size is now a design signal
The current authoritative diff is `+16,349/-521` across 91 files. This is
not 16K lines of production code:
- production main/BE: about `+7,729/-398`;
- FE unit tests: about `+7,100/-122`;
- benchmarks: about `+1,414`;
- regression tests: about `+89/-1`.
The tests are valuable, but 7.7K production additions are still large for
one cache-governance change. In particular:
- `IcebergCacheSizeEstimator`: 1,230 lines;
- `PaimonCacheSizeEstimator`: 592 lines;
- `MetaCacheWeightUtils`: 368 lines.
Those three files alone contain about 2.2K lines devoted mainly to sizing.
That is a warning that the estimator is reproducing implementation details
rather than expressing stable cache semantics.
## Which earlier AI findings should remain
Retain findings that identify a missing **order-of-growth dimension** or a
cache-lifecycle hole:
- partition/file/snapshot/schema counts that can grow without bound;
- partition width (`partitionCount * partitionColumnCount`);
- total schema/type-node, spec-field and sort-field counts;
- alias counts and metadata/property/summary entry counts;
- skew-sensitive string/binary payload already available from the normal
loader;
- coarse reservation for post-admission lazy growth, such as Iceberg schema
indexes or Paimon store-derived RowTypes;
- a child entry independently retaining a frozen table generation;
- arithmetic saturation, once-only publication weighing, and no remote
I/O/lazy materialization for weighing;
- generation-fenced reservation/removal correctness.
These findings should be expressed as a small number of stable logical
cardinalities.
## Which earlier AI suggestions should not drive production code
Do not keep accepting findings whose only purpose is making a hand-written
formula match JOL within 10%:
- exact `HashMap` table/node capacity;
- Guava singleton-versus-regular collection layouts;
- boxed `Integer`/`Long` cache identities;
- precise object alignment and compressed-oops assumptions;
- exact `ByteBuffer`, `BigDecimal`, literal-wrapper or accessor instance
sizes;
- one formula per concrete Paimon `DataType` implementation;
- Iceberg/Paimon private-field layout fingerprints;
- exact counts of lazy lookup maps and accessor wrapper classes;
- rejection of an otherwise supported value only because a future SDK
subtype or private layout is unknown;
- shared-string identity corrections whose only effect is a small JOL-ratio
adjustment.
These are not durable cache invariants. They bind Doris to private details
of Iceberg 1.10.1, Paimon 1.4.2, Guava and the active JVM. They also turn
ordinary SDK upgrades into weighted-cache rejection risks.
An AI review suggestion should be rejected or reframed when it improves a
local byte oracle but violates the system-level constraints: small production
code, stable semantics, bounded hot-path work, no SDK-private coupling, and
maintainable upgrades. The right response to a valid under-count observation is
often "add one logical cardinality term", not "model every Java object it
expands into".
## Recommended production formulas
Use approximate byte-like units, rounded upward to simple constants and
calibrated offline. Document `max-weight` as an estimated retained-cache
admission budget, not an exact heap limit.
```text
Hive partition values
BASE
+ partitionColumnCount * KEY_COLUMN_WEIGHT
+ partitionCount * PARTITION_WEIGHT
+ partitionCount * partitionColumnCount * PARTITION_COLUMN_WEIGHT
+ retainedPartitionPayloadBytes
```
```text
Iceberg table generation
BASE
+ snapshotCount * SNAPSHOT_WEIGHT
+ schemaCount * SCHEMA_WEIGHT
+ totalSchemaFieldCount * FIELD_WEIGHT
+ specCount * SPEC_WEIGHT
+ totalPartitionFieldCount * PARTITION_FIELD_WEIGHT
+ sortOrderCount * SORT_ORDER_WEIGHT
+ totalSortFieldCount * SORT_FIELD_WEIGHT
+ metadataCollectionEntryCount * METADATA_ENTRY_WEIGHT
+ retainedPayloadBytes
```
The lazy schema/spec indexes can be covered by increasing `FIELD_WEIGHT` and
`PARTITION_FIELD_WEIGHT`; they do not require explicit HashMap/accessor models.
```text
Iceberg snapshot projection
BASE
+ partitionCount * PARTITION_WEIGHT
+ partitionCount * partitionColumnCount * PARTITION_COLUMN_WEIGHT
+ aliasCount * ALIAS_WEIGHT
+ nameMappingFieldCount * NAME_MAPPING_FIELD_WEIGHT
+ nameMappingAliasCount * NAME_MAPPING_ALIAS_WEIGHT
+ retainedPartitionPayloadBytes
+ retainedTableGenerationWeight
```
Compute `retainedTableGenerationWeight` once and reuse it. If the table and
snapshot entries independently own the same frozen generation, conservative
duplicate reservation is acceptable for now; do not traverse and estimate the
graph twice. Shared reservation ownership can be a later optimization if
necessary.
```text
Iceberg manifest
BASE
+ dataFileCount * DATA_FILE_WEIGHT
+ deleteFileCount * DELETE_FILE_WEIGHT
+ metricEntryCount * METRIC_ENTRY_WEIGHT
+ retainedPayloadBytes
```
Never read a remote manifest solely to obtain a weight.
```text
Paimon table generation
BASE
+ wrapperCount * WRAPPER_WEIGHT
+ schemaFieldCount * FIELD_WEIGHT
+ logicalTypeNodeCount * TYPE_NODE_WEIGHT
+ optionCount * OPTION_WEIGHT
+ partitionKeyCount * KEY_WEIGHT
+ primaryKeyCount * KEY_WEIGHT
+ bucketKeyCount * KEY_WEIGHT
+ derivedRowTypeCopies * schemaFieldCount * DERIVED_FIELD_WEIGHT
+ retainedPayloadBytes
```
`logicalTypeNodeCount` is a stable schema concept; exact Java class instance
layouts are not. Unknown future logical types should receive a conservative
generic weight instead of disabling weighted caching.
```text
Paimon snapshot projection
BASE
+ partitionCount * PARTITION_WEIGHT
+ partitionCount * partitionColumnCount * PARTITION_COLUMN_WEIGHT
+ retainedPartitionPayloadBytes
+ retainedTableGenerationWeight
```
Do not charge `Partition.fileCount()` as retained files: it is a scalar
unless the value actually owns file objects.
## What JOL should do
Keep JOL, if its license/dependency placement is acceptable, only in a
benchmark/test-only calibration module. Use it to choose upward-rounded
constants and periodically detect order-of-magnitude mistakes. Do not require
every fixture to stay within `[1.00, 1.10]` of JOL.
The production acceptance criteria should instead be:
- monotonic weight for 1K/10K/100K cardinalities;
- major narrow/wide and flat/nested dimensions are represented;
- no remote I/O or lazy materialization during weighing;
- bounded publication CPU/allocation;
- saturated arithmetic;
- hits/removals are O(1);
- soft collection releases only the matching reservation generation;
- typical fixtures have no order-of-magnitude under-estimation.
## Suggested convergence
Please keep the now-hardened admission/lifecycle framework, but replace the
production Iceberg/Paimon structural estimators and SDK field-signature
machinery with the coarse formulas above. The full estimator/JOL fixtures can
remain as offline calibration evidence rather than production correctness
machinery.
This is not a request to discard all prior review feedback. It is a request
to retain the stable semantic dimensions discovered by review while stopping
the object-by-object implementation strategy. Otherwise future
Iceberg/Paimon/JDK upgrades will require repeatedly teaching Doris another
version of third-party private object graphs, even though the mechanism still
cannot bound load-time peak memory before admission.
--
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]
