JackieTien97 commented on code in PR #18409:
URL: https://github.com/apache/iotdb/pull/18409#discussion_r3748229489
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/utils/datastructure/AlignedTVList.java:
##########
@@ -212,39 +260,165 @@ public synchronized AlignedTVList cloneForFlushSort() {
public synchronized AlignedTVList clone() {
AlignedTVList cloneList = AlignedTVList.newAlignedList(new
ArrayList<>(dataTypes));
cloneAs(cloneList);
- cloneList.timeDeletedCnt = this.timeDeletedCnt;
- System.arraycopy(
- memoryBinaryChunkSize, 0, cloneList.memoryBinaryChunkSize, 0,
dataTypes.size());
- for (int i = 0; i < values.size(); i++) {
- // Clone value
+ cloneColumnDataTo(cloneList, null);
+ cloneList.materializedValueArrayCounts =
+ Arrays.copyOf(materializedValueArrayCounts,
materializedValueArrayCounts.length);
+ cloneList.materializedValueArrayMemCost = materializedValueArrayMemCost;
+ return cloneList;
+ }
+
+ /**
+ * Prepare a partial clone without changing this TVList. The returned plan
must be committed only
+ * after the query-memory reservation succeeds.
+ */
+ public synchronized PartialClonePlan preparePartialClone(Set<Integer>
columnsToClone) {
+ Set<Integer> retainedColumns =
+ new HashSet<>(Objects.requireNonNull(columnsToClone, "columnsToClone
cannot be null"));
+ AlignedTVList cloneList = AlignedTVList.newAlignedList(new
ArrayList<>(dataTypes));
+ cloneAs(cloneList);
+ cloneColumnDataTo(cloneList, retainedColumns);
+ return prepareMovePlan(cloneList, retainedColumns);
+ }
+
+ @SuppressWarnings("unchecked")
+ private PartialClonePlan prepareMovePlan(AlignedTVList cloneList,
Set<Integer> retainedColumns) {
+ Objects.requireNonNull(cloneList, "cloneList cannot be null");
+ int columnCount = values.size();
+ if (cloneList.values.size() != columnCount
+ || cloneList.memoryBinaryChunkSize.length !=
memoryBinaryChunkSize.length) {
+ throw new IllegalStateException("Target AlignedTVList has incompatible
column containers");
+ }
+
+ List<Object>[] valueColumnsToMove = (List<Object>[]) new
List<?>[columnCount];
Review Comment:
**[P1] Cover the O(N) partial-clone preparation peak before allocating it**
The caller reserves only the final retained-source size before entering this
method. However, `newAlignedList` has already created an inner `ArrayList` for
every column (potentially preallocating `getDefaultArrayNum()` slots), and
these two N-entry move arrays are allocated as additional preparation state.
For every non-retained column, the clone's empty inner list is replaced during
commit and becomes garbage, so this transient O(N) allocation is absent from
both the final source size and the existing reservation. This remains true even
after adding the retained-container cost requested in the earlier thread, so
the claim that the transient clone increase is protected by admission control
is not currently valid for wide aligned devices.
Please either construct a partial clone with null outer slots and allocate
inner lists only for retained columns, or estimate and reserve the preparation
overhead separately and release that temporary reservation after
commit/rollback. Add a wide sparse-column test that exercises this
peak-accounting path.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/utils/datastructure/AlignedTVList.java:
##########
@@ -1137,22 +1410,51 @@ public synchronized RamInfo calculateRamSize() {
new ArrayList<>(dataTypes));
}
+ public synchronized RamInfo calculateRamSize(Set<Integer> columnsToClone) {
+ return new RamInfo(
+ timestamps.size(),
+ alignedTvListArrayMemCost(columnsToClone),
+ getRamSize(columnsToClone),
+ rowCount,
+ new ArrayList<>(dataTypes));
+ }
+
public synchronized long getRamSize() {
return (long) timestamps.size() *
alignedTvListArrayMemCostWithoutPrimitiveArrays()
+ materializedValueArrayMemCost
+ materializedBitmapMemoryCost;
}
- private static long calculateBitmapRamCost(List<List<BitMap>> bitMaps) {
+ public synchronized long getRamSize(Set<Integer> columnsToClone) {
+ long size =
+ (long) timestamps.size() *
alignedTvListArrayMemCostWithoutPrimitiveArrays(columnsToClone);
+ for (int i = 0; i < dataTypes.size(); i++) {
+ if (columnsToClone != null && !columnsToClone.contains(i)) {
+ continue;
+ }
+ TSDataType dataType = dataTypes.get(i);
+ if (dataType != null) {
+ size += (long) materializedValueArrayCounts[i] *
valueListArrayMemCost(dataType);
Review Comment:
**[P1] Count lazy null backing slots independently of materialized arrays**
On master, a retained value column can have B block entries in its inner
`ArrayList` but only M materialized primitive arrays. This line charges
`NUM_BYTES_OBJECT_REF` only M times via `valueListArrayMemCost`; meanwhile
`alignedTvListArrayMemCostWithoutPrimitiveArrays` subtracts that per-column
reference for every block. The B-M null slots in the inner backing `Object[]`
are therefore uncharged. Porting dev/1.3's `calculateContainerRamCost` verbatim
would not fix this, because its `listRamCostWithoutReferences` intentionally
omits the backing-array references.
Please adapt the container calculation to master's lazy layout so every
inner-list slot is counted exactly once while primitive payload/header cost
remains based on the materialized count. Add a sparse multi-block case where a
retained column has both materialized and all-null blocks; it should verify the
calculated retained size as well as reservation/cleanup equality.
--
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]
