You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "HTHou (via GitHub)" <gi...@apache.org> on 2024/06/12 08:08:05 UTC
[PR] Fix memory calculate error when insertRecords with both aligned and non-aligned devices [iotdb]
HTHou opened a new pull request, #12720:
URL: https://github.com/apache/iotdb/pull/12720
## Description
For an InsertRowsNode, we cannot ensure it only contains one alignment type of InsertRowNodes.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix memory calculate error when insertRecords with both aligned and non-aligned devices [iotdb]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #12720:
URL: https://github.com/apache/iotdb/pull/12720#issuecomment-2162474432
## [Codecov](https://app.codecov.io/gh/apache/iotdb/pull/12720?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `94.38202%` with `5 lines` in your changes missing coverage. Please review.
> Project coverage is 42.09%. Comparing base [(`a9d1401`)](https://app.codecov.io/gh/apache/iotdb/commit/a9d1401881231ed2e6fe6482b48c829b2237d5c6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`459db93`)](https://app.codecov.io/gh/apache/iotdb/commit/459db93fa925af74bc189bdf59729be02406d988?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 4 commits behind head on master.
| [Files](https://app.codecov.io/gh/apache/iotdb/pull/12720?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ageengine/dataregion/memtable/TsFileProcessor.java](https://app.codecov.io/gh/apache/iotdb/pull/12720?src=pr&el=tree&filepath=iotdb-core%2Fdatanode%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fiotdb%2Fdb%2Fstorageengine%2Fdataregion%2Fmemtable%2FTsFileProcessor.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aW90ZGItY29yZS9kYXRhbm9kZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaW90ZGIvZGIvc3RvcmFnZWVuZ2luZS9kYXRhcmVnaW9uL21lbXRhYmxlL1RzRmlsZVByb2Nlc3Nvci5qYXZh) | 94.38% | [5 Missing :warning: ](https://app.codecov.io/gh/apache/iotdb/pull/12720?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #12720 +/- ##
============================================
- Coverage 42.09% 42.09% -0.01%
Complexity 71 71
============================================
Files 3260 3260
Lines 205703 205709 +6
Branches 24525 24524 -1
============================================
- Hits 86599 86592 -7
- Misses 119104 119117 +13
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/iotdb/pull/12720?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix memory calculate error when insertRecords with both aligned and non-aligned devices [iotdb]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #12720:
URL: https://github.com/apache/iotdb/pull/12720#issuecomment-2350799221
## [](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=12720) **Quality Gate passed**
Issues
 [17 New issues](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=12720&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=12720&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_iotdb&pullRequest=12720&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=12720&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=12720&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=12720)
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix memory calculate error when insertRecords with both aligned and non-aligned devices [iotdb]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #12720:
URL: https://github.com/apache/iotdb/pull/12720#issuecomment-2162388539
## [](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=12720) **Quality Gate failed**
Failed conditions
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=12720&metric=new_coverage&view=list) (required ≥ 80%)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=12720)
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix memory calculate error when insertRecords with both aligned and non-aligned devices [iotdb]
Posted by "jt2594838 (via GitHub)" <gi...@apache.org>.
jt2594838 commented on code in PR #12720:
URL: https://github.com/apache/iotdb/pull/12720#discussion_r1639355843
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/memtable/TsFileProcessor.java:
##########
@@ -561,55 +557,163 @@ private long[] checkMemCostAndAddToTspInfoForRow(
return new long[] {memTableIncrement, textDataIncrement, chunkMetadataIncrement};
}
- @SuppressWarnings("squid:S3776") // High Cognitive Complexity
private long[] checkMemCostAndAddToTspInfoForRows(InsertRowsNode insertRowsNode)
throws WriteProcessException {
- // Memory of increased PrimitiveArray and TEXT values, e.g., add a long[128], add 128*8
- long memTableIncrement = 0L;
- long textDataIncrement = 0L;
- long chunkMetadataIncrement = 0L;
+
+ long[] memIncrements = new long[3];
// device -> measurement -> adding TVList size
- Map<IDeviceID, Map<String, Integer>> increasingMemTableInfo = new HashMap<>();
+ Map<IDeviceID, Map<String, Integer>> increasingMemTableInfoForNonAligned = new HashMap<>();
+ // device -> (measurements -> datatype, adding aligned TVList size)
+ Map<IDeviceID, Pair<Map<String, TSDataType>, Integer>> increasingMemTableInfoForAligned =
+ new HashMap<>();
for (InsertRowNode insertRowNode : insertRowsNode.getInsertRowNodeList()) {
- IDeviceID deviceId = insertRowNode.getDeviceID();
- TSDataType[] dataTypes = insertRowNode.getDataTypes();
- Object[] values = insertRowNode.getValues();
- String[] measurements = insertRowNode.getMeasurements();
+ if (insertRowNode.isAligned()) {
+ handleAlignedData(insertRowNode, memIncrements, increasingMemTableInfoForAligned);
+ } else {
+ handleUnalignedData(insertRowNode, memIncrements, increasingMemTableInfoForNonAligned);
+ }
+ }
+ updateMemoryInfo(memIncrements[0], memIncrements[2], memIncrements[1]);
+ return memIncrements;
+ }
+
+ @SuppressWarnings("squid:S3776") // High Cognitive Complexity
+ private void handleAlignedData(
+ InsertRowNode insertRowNode,
+ long[] memIncrements,
+ Map<IDeviceID, Pair<Map<String, TSDataType>, Integer>> increasingMemTableInfoForAligned) {
+ long memTableIncrement = memIncrements[0];
+ long textDataIncrement = memIncrements[1];
+ long chunkMetadataIncrement = memIncrements[2];
+
+ IDeviceID deviceId = insertRowNode.getDeviceID();
+ TSDataType[] dataTypes = insertRowNode.getDataTypes();
+ Object[] values = insertRowNode.getValues();
+ String[] measurements = insertRowNode.getMeasurements();
+
+ if (workMemTable.checkIfChunkDoesNotExist(deviceId, AlignedPath.VECTOR_PLACEHOLDER)
+ && !increasingMemTableInfoForAligned.containsKey(deviceId)) {
+ // For new device of this mem table
+ // ChunkMetadataIncrement
+ chunkMetadataIncrement +=
+ ChunkMetadata.calculateRamSize(AlignedPath.VECTOR_PLACEHOLDER, TSDataType.VECTOR)
+ * dataTypes.length;
+ memTableIncrement += AlignedTVList.alignedTvListArrayMemCost(dataTypes);
for (int i = 0; i < dataTypes.length; i++) {
// Skip failed Measurements
if (dataTypes[i] == null || measurements[i] == null) {
continue;
}
- if (workMemTable.checkIfChunkDoesNotExist(deviceId, measurements[i])
- && (!increasingMemTableInfo.containsKey(deviceId)
- || !increasingMemTableInfo.get(deviceId).containsKey(measurements[i]))) {
- // ChunkMetadataIncrement
- chunkMetadataIncrement += ChunkMetadata.calculateRamSize(measurements[i], dataTypes[i]);
- memTableIncrement += TVList.tvListArrayMemCost(dataTypes[i]);
- increasingMemTableInfo
- .computeIfAbsent(deviceId, k -> new HashMap<>())
- .putIfAbsent(measurements[i], 1);
- } else {
- // here currentChunkPointNum >= 1
- long currentChunkPointNum = workMemTable.getCurrentTVListSize(deviceId, measurements[i]);
- int addingPointNum =
- increasingMemTableInfo
- .computeIfAbsent(deviceId, k -> new HashMap<>())
- .computeIfAbsent(measurements[i], k -> 0);
+ increasingMemTableInfoForAligned
+ .computeIfAbsent(deviceId, k -> new Pair<>(new HashMap<>(), 1))
+ .left
+ .put(measurements[i], dataTypes[i]);
+ // TEXT data mem size
+ if (dataTypes[i] == TSDataType.TEXT && values[i] != null) {
+ textDataIncrement += MemUtils.getBinarySize((Binary) values[i]);
+ }
+ }
+
+ } else {
+ // For existed device of this mem table
+ AlignedWritableMemChunkGroup memChunkGroup =
+ (AlignedWritableMemChunkGroup) workMemTable.getMemTableMap().get(deviceId);
+ AlignedWritableMemChunk alignedMemChunk =
+ memChunkGroup == null ? null : memChunkGroup.getAlignedMemChunk();
+ long currentChunkPointNum = alignedMemChunk == null ? 0 : alignedMemChunk.alignedListSize();
+ List<TSDataType> dataTypesInTVList = new ArrayList<>();
+ Pair<Map<String, TSDataType>, Integer> addingPointNumInfo =
+ increasingMemTableInfoForAligned.computeIfAbsent(
+ deviceId, k -> new Pair<>(new HashMap<>(), 0));
+ for (int i = 0; i < dataTypes.length; i++) {
+ // Skip failed Measurements
+ if (dataTypes[i] == null || measurements[i] == null) {
+ continue;
+ }
+
+ int addingPointNum = addingPointNumInfo.getRight();
+ // Extending the column of aligned mem chunk
+ if ((alignedMemChunk != null && !alignedMemChunk.containsMeasurement(measurements[i]))
+ && !increasingMemTableInfoForAligned.get(deviceId).left.containsKey(measurements[i])) {
memTableIncrement +=
- ((currentChunkPointNum + addingPointNum) % PrimitiveArrayManager.ARRAY_SIZE) == 0
- ? TVList.tvListArrayMemCost(dataTypes[i])
- : 0;
- increasingMemTableInfo.get(deviceId).computeIfPresent(measurements[i], (k, v) -> v + 1);
+ ((currentChunkPointNum + addingPointNum) / PrimitiveArrayManager.ARRAY_SIZE + 1)
+ * AlignedTVList.valueListArrayMemCost(dataTypes[i]);
Review Comment:
This does not seem like "increment".
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/memtable/TsFileProcessor.java:
##########
@@ -561,55 +557,163 @@ private long[] checkMemCostAndAddToTspInfoForRow(
return new long[] {memTableIncrement, textDataIncrement, chunkMetadataIncrement};
}
- @SuppressWarnings("squid:S3776") // High Cognitive Complexity
private long[] checkMemCostAndAddToTspInfoForRows(InsertRowsNode insertRowsNode)
throws WriteProcessException {
- // Memory of increased PrimitiveArray and TEXT values, e.g., add a long[128], add 128*8
- long memTableIncrement = 0L;
- long textDataIncrement = 0L;
- long chunkMetadataIncrement = 0L;
+
+ long[] memIncrements = new long[3];
// device -> measurement -> adding TVList size
- Map<IDeviceID, Map<String, Integer>> increasingMemTableInfo = new HashMap<>();
+ Map<IDeviceID, Map<String, Integer>> increasingMemTableInfoForNonAligned = new HashMap<>();
+ // device -> (measurements -> datatype, adding aligned TVList size)
+ Map<IDeviceID, Pair<Map<String, TSDataType>, Integer>> increasingMemTableInfoForAligned =
+ new HashMap<>();
for (InsertRowNode insertRowNode : insertRowsNode.getInsertRowNodeList()) {
- IDeviceID deviceId = insertRowNode.getDeviceID();
- TSDataType[] dataTypes = insertRowNode.getDataTypes();
- Object[] values = insertRowNode.getValues();
- String[] measurements = insertRowNode.getMeasurements();
+ if (insertRowNode.isAligned()) {
+ handleAlignedData(insertRowNode, memIncrements, increasingMemTableInfoForAligned);
+ } else {
+ handleUnalignedData(insertRowNode, memIncrements, increasingMemTableInfoForNonAligned);
+ }
+ }
+ updateMemoryInfo(memIncrements[0], memIncrements[2], memIncrements[1]);
+ return memIncrements;
+ }
+
+ @SuppressWarnings("squid:S3776") // High Cognitive Complexity
+ private void handleAlignedData(
+ InsertRowNode insertRowNode,
+ long[] memIncrements,
+ Map<IDeviceID, Pair<Map<String, TSDataType>, Integer>> increasingMemTableInfoForAligned) {
+ long memTableIncrement = memIncrements[0];
+ long textDataIncrement = memIncrements[1];
+ long chunkMetadataIncrement = memIncrements[2];
+
+ IDeviceID deviceId = insertRowNode.getDeviceID();
+ TSDataType[] dataTypes = insertRowNode.getDataTypes();
+ Object[] values = insertRowNode.getValues();
+ String[] measurements = insertRowNode.getMeasurements();
+
+ if (workMemTable.checkIfChunkDoesNotExist(deviceId, AlignedPath.VECTOR_PLACEHOLDER)
+ && !increasingMemTableInfoForAligned.containsKey(deviceId)) {
+ // For new device of this mem table
+ // ChunkMetadataIncrement
+ chunkMetadataIncrement +=
+ ChunkMetadata.calculateRamSize(AlignedPath.VECTOR_PLACEHOLDER, TSDataType.VECTOR)
+ * dataTypes.length;
+ memTableIncrement += AlignedTVList.alignedTvListArrayMemCost(dataTypes);
for (int i = 0; i < dataTypes.length; i++) {
// Skip failed Measurements
if (dataTypes[i] == null || measurements[i] == null) {
continue;
}
- if (workMemTable.checkIfChunkDoesNotExist(deviceId, measurements[i])
- && (!increasingMemTableInfo.containsKey(deviceId)
- || !increasingMemTableInfo.get(deviceId).containsKey(measurements[i]))) {
- // ChunkMetadataIncrement
- chunkMetadataIncrement += ChunkMetadata.calculateRamSize(measurements[i], dataTypes[i]);
- memTableIncrement += TVList.tvListArrayMemCost(dataTypes[i]);
- increasingMemTableInfo
- .computeIfAbsent(deviceId, k -> new HashMap<>())
- .putIfAbsent(measurements[i], 1);
- } else {
- // here currentChunkPointNum >= 1
- long currentChunkPointNum = workMemTable.getCurrentTVListSize(deviceId, measurements[i]);
- int addingPointNum =
- increasingMemTableInfo
- .computeIfAbsent(deviceId, k -> new HashMap<>())
- .computeIfAbsent(measurements[i], k -> 0);
+ increasingMemTableInfoForAligned
+ .computeIfAbsent(deviceId, k -> new Pair<>(new HashMap<>(), 1))
+ .left
+ .put(measurements[i], dataTypes[i]);
+ // TEXT data mem size
+ if (dataTypes[i] == TSDataType.TEXT && values[i] != null) {
+ textDataIncrement += MemUtils.getBinarySize((Binary) values[i]);
+ }
+ }
+
+ } else {
+ // For existed device of this mem table
+ AlignedWritableMemChunkGroup memChunkGroup =
+ (AlignedWritableMemChunkGroup) workMemTable.getMemTableMap().get(deviceId);
+ AlignedWritableMemChunk alignedMemChunk =
+ memChunkGroup == null ? null : memChunkGroup.getAlignedMemChunk();
+ long currentChunkPointNum = alignedMemChunk == null ? 0 : alignedMemChunk.alignedListSize();
+ List<TSDataType> dataTypesInTVList = new ArrayList<>();
+ Pair<Map<String, TSDataType>, Integer> addingPointNumInfo =
+ increasingMemTableInfoForAligned.computeIfAbsent(
+ deviceId, k -> new Pair<>(new HashMap<>(), 0));
+ for (int i = 0; i < dataTypes.length; i++) {
+ // Skip failed Measurements
+ if (dataTypes[i] == null || measurements[i] == null) {
+ continue;
+ }
+
+ int addingPointNum = addingPointNumInfo.getRight();
+ // Extending the column of aligned mem chunk
+ if ((alignedMemChunk != null && !alignedMemChunk.containsMeasurement(measurements[i]))
+ && !increasingMemTableInfoForAligned.get(deviceId).left.containsKey(measurements[i])) {
memTableIncrement +=
- ((currentChunkPointNum + addingPointNum) % PrimitiveArrayManager.ARRAY_SIZE) == 0
- ? TVList.tvListArrayMemCost(dataTypes[i])
- : 0;
- increasingMemTableInfo.get(deviceId).computeIfPresent(measurements[i], (k, v) -> v + 1);
+ ((currentChunkPointNum + addingPointNum) / PrimitiveArrayManager.ARRAY_SIZE + 1)
+ * AlignedTVList.valueListArrayMemCost(dataTypes[i]);
+ increasingMemTableInfoForAligned.get(deviceId).left.put(measurements[i], dataTypes[i]);
}
// TEXT data mem size
- if (dataTypes[i].isBinary() && values[i] != null) {
+ if (dataTypes[i] == TSDataType.TEXT && values[i] != null) {
textDataIncrement += MemUtils.getBinarySize((Binary) values[i]);
}
}
+ int addingPointNum = increasingMemTableInfoForAligned.get(deviceId).getRight();
+ // Here currentChunkPointNum + addingPointNum >= 1
+ if (((currentChunkPointNum + addingPointNum) % PrimitiveArrayManager.ARRAY_SIZE) == 0) {
+ if (alignedMemChunk != null) {
+ dataTypesInTVList.addAll(((AlignedTVList) alignedMemChunk.getTVList()).getTsDataTypes());
+ }
+ dataTypesInTVList.addAll(increasingMemTableInfoForAligned.get(deviceId).getLeft().values());
+ memTableIncrement += AlignedTVList.alignedTvListArrayMemCost(dataTypesInTVList);
Review Comment:
Will the calculation repeat the one in line 638?
For example, if "s3" is a new measurement of the device, its size will be calculated in line 638 and here again.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/memtable/TsFileProcessor.java:
##########
@@ -561,55 +557,163 @@ private long[] checkMemCostAndAddToTspInfoForRow(
return new long[] {memTableIncrement, textDataIncrement, chunkMetadataIncrement};
}
- @SuppressWarnings("squid:S3776") // High Cognitive Complexity
private long[] checkMemCostAndAddToTspInfoForRows(InsertRowsNode insertRowsNode)
throws WriteProcessException {
- // Memory of increased PrimitiveArray and TEXT values, e.g., add a long[128], add 128*8
- long memTableIncrement = 0L;
- long textDataIncrement = 0L;
- long chunkMetadataIncrement = 0L;
+
+ long[] memIncrements = new long[3];
// device -> measurement -> adding TVList size
- Map<IDeviceID, Map<String, Integer>> increasingMemTableInfo = new HashMap<>();
+ Map<IDeviceID, Map<String, Integer>> increasingMemTableInfoForNonAligned = new HashMap<>();
+ // device -> (measurements -> datatype, adding aligned TVList size)
+ Map<IDeviceID, Pair<Map<String, TSDataType>, Integer>> increasingMemTableInfoForAligned =
+ new HashMap<>();
for (InsertRowNode insertRowNode : insertRowsNode.getInsertRowNodeList()) {
- IDeviceID deviceId = insertRowNode.getDeviceID();
- TSDataType[] dataTypes = insertRowNode.getDataTypes();
- Object[] values = insertRowNode.getValues();
- String[] measurements = insertRowNode.getMeasurements();
+ if (insertRowNode.isAligned()) {
+ handleAlignedData(insertRowNode, memIncrements, increasingMemTableInfoForAligned);
+ } else {
+ handleUnalignedData(insertRowNode, memIncrements, increasingMemTableInfoForNonAligned);
+ }
+ }
+ updateMemoryInfo(memIncrements[0], memIncrements[2], memIncrements[1]);
+ return memIncrements;
+ }
+
+ @SuppressWarnings("squid:S3776") // High Cognitive Complexity
+ private void handleAlignedData(
+ InsertRowNode insertRowNode,
+ long[] memIncrements,
+ Map<IDeviceID, Pair<Map<String, TSDataType>, Integer>> increasingMemTableInfoForAligned) {
+ long memTableIncrement = memIncrements[0];
+ long textDataIncrement = memIncrements[1];
+ long chunkMetadataIncrement = memIncrements[2];
+
+ IDeviceID deviceId = insertRowNode.getDeviceID();
+ TSDataType[] dataTypes = insertRowNode.getDataTypes();
+ Object[] values = insertRowNode.getValues();
+ String[] measurements = insertRowNode.getMeasurements();
+
+ if (workMemTable.checkIfChunkDoesNotExist(deviceId, AlignedPath.VECTOR_PLACEHOLDER)
+ && !increasingMemTableInfoForAligned.containsKey(deviceId)) {
+ // For new device of this mem table
+ // ChunkMetadataIncrement
+ chunkMetadataIncrement +=
+ ChunkMetadata.calculateRamSize(AlignedPath.VECTOR_PLACEHOLDER, TSDataType.VECTOR)
+ * dataTypes.length;
+ memTableIncrement += AlignedTVList.alignedTvListArrayMemCost(dataTypes);
for (int i = 0; i < dataTypes.length; i++) {
// Skip failed Measurements
if (dataTypes[i] == null || measurements[i] == null) {
continue;
}
- if (workMemTable.checkIfChunkDoesNotExist(deviceId, measurements[i])
- && (!increasingMemTableInfo.containsKey(deviceId)
- || !increasingMemTableInfo.get(deviceId).containsKey(measurements[i]))) {
- // ChunkMetadataIncrement
- chunkMetadataIncrement += ChunkMetadata.calculateRamSize(measurements[i], dataTypes[i]);
- memTableIncrement += TVList.tvListArrayMemCost(dataTypes[i]);
- increasingMemTableInfo
- .computeIfAbsent(deviceId, k -> new HashMap<>())
- .putIfAbsent(measurements[i], 1);
- } else {
- // here currentChunkPointNum >= 1
- long currentChunkPointNum = workMemTable.getCurrentTVListSize(deviceId, measurements[i]);
- int addingPointNum =
- increasingMemTableInfo
- .computeIfAbsent(deviceId, k -> new HashMap<>())
- .computeIfAbsent(measurements[i], k -> 0);
+ increasingMemTableInfoForAligned
+ .computeIfAbsent(deviceId, k -> new Pair<>(new HashMap<>(), 1))
+ .left
+ .put(measurements[i], dataTypes[i]);
+ // TEXT data mem size
+ if (dataTypes[i] == TSDataType.TEXT && values[i] != null) {
+ textDataIncrement += MemUtils.getBinarySize((Binary) values[i]);
+ }
+ }
+
+ } else {
+ // For existed device of this mem table
+ AlignedWritableMemChunkGroup memChunkGroup =
+ (AlignedWritableMemChunkGroup) workMemTable.getMemTableMap().get(deviceId);
+ AlignedWritableMemChunk alignedMemChunk =
+ memChunkGroup == null ? null : memChunkGroup.getAlignedMemChunk();
+ long currentChunkPointNum = alignedMemChunk == null ? 0 : alignedMemChunk.alignedListSize();
+ List<TSDataType> dataTypesInTVList = new ArrayList<>();
+ Pair<Map<String, TSDataType>, Integer> addingPointNumInfo =
+ increasingMemTableInfoForAligned.computeIfAbsent(
+ deviceId, k -> new Pair<>(new HashMap<>(), 0));
+ for (int i = 0; i < dataTypes.length; i++) {
+ // Skip failed Measurements
+ if (dataTypes[i] == null || measurements[i] == null) {
+ continue;
+ }
+
+ int addingPointNum = addingPointNumInfo.getRight();
+ // Extending the column of aligned mem chunk
+ if ((alignedMemChunk != null && !alignedMemChunk.containsMeasurement(measurements[i]))
+ && !increasingMemTableInfoForAligned.get(deviceId).left.containsKey(measurements[i])) {
memTableIncrement +=
- ((currentChunkPointNum + addingPointNum) % PrimitiveArrayManager.ARRAY_SIZE) == 0
- ? TVList.tvListArrayMemCost(dataTypes[i])
- : 0;
- increasingMemTableInfo.get(deviceId).computeIfPresent(measurements[i], (k, v) -> v + 1);
+ ((currentChunkPointNum + addingPointNum) / PrimitiveArrayManager.ARRAY_SIZE + 1)
+ * AlignedTVList.valueListArrayMemCost(dataTypes[i]);
Review Comment:
If (currentChunkPointNum + addingPointNum) == PrimitiveArrayManager.ARRAY_SIZE, the calculation may be imprecise.
In the worst case, where each batch contains exactly PrimitiveArrayManager.ARRAY_SIZE rows, the estimation can be doubled.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/memtable/TsFileProcessor.java:
##########
@@ -561,55 +557,163 @@ private long[] checkMemCostAndAddToTspInfoForRow(
return new long[] {memTableIncrement, textDataIncrement, chunkMetadataIncrement};
}
- @SuppressWarnings("squid:S3776") // High Cognitive Complexity
private long[] checkMemCostAndAddToTspInfoForRows(InsertRowsNode insertRowsNode)
throws WriteProcessException {
- // Memory of increased PrimitiveArray and TEXT values, e.g., add a long[128], add 128*8
- long memTableIncrement = 0L;
- long textDataIncrement = 0L;
- long chunkMetadataIncrement = 0L;
+
+ long[] memIncrements = new long[3];
// device -> measurement -> adding TVList size
- Map<IDeviceID, Map<String, Integer>> increasingMemTableInfo = new HashMap<>();
+ Map<IDeviceID, Map<String, Integer>> increasingMemTableInfoForNonAligned = new HashMap<>();
+ // device -> (measurements -> datatype, adding aligned TVList size)
+ Map<IDeviceID, Pair<Map<String, TSDataType>, Integer>> increasingMemTableInfoForAligned =
+ new HashMap<>();
for (InsertRowNode insertRowNode : insertRowsNode.getInsertRowNodeList()) {
- IDeviceID deviceId = insertRowNode.getDeviceID();
- TSDataType[] dataTypes = insertRowNode.getDataTypes();
- Object[] values = insertRowNode.getValues();
- String[] measurements = insertRowNode.getMeasurements();
+ if (insertRowNode.isAligned()) {
+ handleAlignedData(insertRowNode, memIncrements, increasingMemTableInfoForAligned);
+ } else {
+ handleUnalignedData(insertRowNode, memIncrements, increasingMemTableInfoForNonAligned);
+ }
+ }
+ updateMemoryInfo(memIncrements[0], memIncrements[2], memIncrements[1]);
+ return memIncrements;
+ }
+
+ @SuppressWarnings("squid:S3776") // High Cognitive Complexity
+ private void handleAlignedData(
Review Comment:
The name is too broad. Something like `calculateAlignedRowMemory` woule be better.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/memtable/TsFileProcessor.java:
##########
@@ -561,55 +557,163 @@ private long[] checkMemCostAndAddToTspInfoForRow(
return new long[] {memTableIncrement, textDataIncrement, chunkMetadataIncrement};
}
- @SuppressWarnings("squid:S3776") // High Cognitive Complexity
private long[] checkMemCostAndAddToTspInfoForRows(InsertRowsNode insertRowsNode)
throws WriteProcessException {
- // Memory of increased PrimitiveArray and TEXT values, e.g., add a long[128], add 128*8
- long memTableIncrement = 0L;
- long textDataIncrement = 0L;
- long chunkMetadataIncrement = 0L;
+
+ long[] memIncrements = new long[3];
// device -> measurement -> adding TVList size
- Map<IDeviceID, Map<String, Integer>> increasingMemTableInfo = new HashMap<>();
+ Map<IDeviceID, Map<String, Integer>> increasingMemTableInfoForNonAligned = new HashMap<>();
+ // device -> (measurements -> datatype, adding aligned TVList size)
+ Map<IDeviceID, Pair<Map<String, TSDataType>, Integer>> increasingMemTableInfoForAligned =
+ new HashMap<>();
for (InsertRowNode insertRowNode : insertRowsNode.getInsertRowNodeList()) {
- IDeviceID deviceId = insertRowNode.getDeviceID();
- TSDataType[] dataTypes = insertRowNode.getDataTypes();
- Object[] values = insertRowNode.getValues();
- String[] measurements = insertRowNode.getMeasurements();
+ if (insertRowNode.isAligned()) {
+ handleAlignedData(insertRowNode, memIncrements, increasingMemTableInfoForAligned);
+ } else {
+ handleUnalignedData(insertRowNode, memIncrements, increasingMemTableInfoForNonAligned);
+ }
+ }
+ updateMemoryInfo(memIncrements[0], memIncrements[2], memIncrements[1]);
+ return memIncrements;
+ }
+
+ @SuppressWarnings("squid:S3776") // High Cognitive Complexity
+ private void handleAlignedData(
+ InsertRowNode insertRowNode,
+ long[] memIncrements,
+ Map<IDeviceID, Pair<Map<String, TSDataType>, Integer>> increasingMemTableInfoForAligned) {
+ long memTableIncrement = memIncrements[0];
+ long textDataIncrement = memIncrements[1];
+ long chunkMetadataIncrement = memIncrements[2];
+
+ IDeviceID deviceId = insertRowNode.getDeviceID();
+ TSDataType[] dataTypes = insertRowNode.getDataTypes();
+ Object[] values = insertRowNode.getValues();
+ String[] measurements = insertRowNode.getMeasurements();
+
+ if (workMemTable.checkIfChunkDoesNotExist(deviceId, AlignedPath.VECTOR_PLACEHOLDER)
+ && !increasingMemTableInfoForAligned.containsKey(deviceId)) {
+ // For new device of this mem table
+ // ChunkMetadataIncrement
+ chunkMetadataIncrement +=
+ ChunkMetadata.calculateRamSize(AlignedPath.VECTOR_PLACEHOLDER, TSDataType.VECTOR)
+ * dataTypes.length;
+ memTableIncrement += AlignedTVList.alignedTvListArrayMemCost(dataTypes);
for (int i = 0; i < dataTypes.length; i++) {
// Skip failed Measurements
if (dataTypes[i] == null || measurements[i] == null) {
continue;
}
- if (workMemTable.checkIfChunkDoesNotExist(deviceId, measurements[i])
- && (!increasingMemTableInfo.containsKey(deviceId)
- || !increasingMemTableInfo.get(deviceId).containsKey(measurements[i]))) {
- // ChunkMetadataIncrement
- chunkMetadataIncrement += ChunkMetadata.calculateRamSize(measurements[i], dataTypes[i]);
- memTableIncrement += TVList.tvListArrayMemCost(dataTypes[i]);
- increasingMemTableInfo
- .computeIfAbsent(deviceId, k -> new HashMap<>())
- .putIfAbsent(measurements[i], 1);
- } else {
- // here currentChunkPointNum >= 1
- long currentChunkPointNum = workMemTable.getCurrentTVListSize(deviceId, measurements[i]);
- int addingPointNum =
- increasingMemTableInfo
- .computeIfAbsent(deviceId, k -> new HashMap<>())
- .computeIfAbsent(measurements[i], k -> 0);
+ increasingMemTableInfoForAligned
+ .computeIfAbsent(deviceId, k -> new Pair<>(new HashMap<>(), 1))
+ .left
+ .put(measurements[i], dataTypes[i]);
+ // TEXT data mem size
+ if (dataTypes[i] == TSDataType.TEXT && values[i] != null) {
+ textDataIncrement += MemUtils.getBinarySize((Binary) values[i]);
+ }
+ }
+
+ } else {
+ // For existed device of this mem table
+ AlignedWritableMemChunkGroup memChunkGroup =
+ (AlignedWritableMemChunkGroup) workMemTable.getMemTableMap().get(deviceId);
+ AlignedWritableMemChunk alignedMemChunk =
+ memChunkGroup == null ? null : memChunkGroup.getAlignedMemChunk();
+ long currentChunkPointNum = alignedMemChunk == null ? 0 : alignedMemChunk.alignedListSize();
+ List<TSDataType> dataTypesInTVList = new ArrayList<>();
+ Pair<Map<String, TSDataType>, Integer> addingPointNumInfo =
+ increasingMemTableInfoForAligned.computeIfAbsent(
+ deviceId, k -> new Pair<>(new HashMap<>(), 0));
+ for (int i = 0; i < dataTypes.length; i++) {
+ // Skip failed Measurements
+ if (dataTypes[i] == null || measurements[i] == null) {
+ continue;
+ }
+
+ int addingPointNum = addingPointNumInfo.getRight();
+ // Extending the column of aligned mem chunk
+ if ((alignedMemChunk != null && !alignedMemChunk.containsMeasurement(measurements[i]))
+ && !increasingMemTableInfoForAligned.get(deviceId).left.containsKey(measurements[i])) {
memTableIncrement +=
- ((currentChunkPointNum + addingPointNum) % PrimitiveArrayManager.ARRAY_SIZE) == 0
- ? TVList.tvListArrayMemCost(dataTypes[i])
- : 0;
- increasingMemTableInfo.get(deviceId).computeIfPresent(measurements[i], (k, v) -> v + 1);
+ ((currentChunkPointNum + addingPointNum) / PrimitiveArrayManager.ARRAY_SIZE + 1)
+ * AlignedTVList.valueListArrayMemCost(dataTypes[i]);
+ increasingMemTableInfoForAligned.get(deviceId).left.put(measurements[i], dataTypes[i]);
}
// TEXT data mem size
- if (dataTypes[i].isBinary() && values[i] != null) {
+ if (dataTypes[i] == TSDataType.TEXT && values[i] != null) {
textDataIncrement += MemUtils.getBinarySize((Binary) values[i]);
}
}
+ int addingPointNum = increasingMemTableInfoForAligned.get(deviceId).getRight();
+ // Here currentChunkPointNum + addingPointNum >= 1
+ if (((currentChunkPointNum + addingPointNum) % PrimitiveArrayManager.ARRAY_SIZE) == 0) {
Review Comment:
In lines 623 and 627, you initialize the `currentChunkPointNum` and `addingPointNum` to 0. They have not changed afterward. So it is possible that `currentChunkPointNum + addingPointNum == 0`.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/memtable/TsFileProcessor.java:
##########
@@ -561,55 +557,163 @@ private long[] checkMemCostAndAddToTspInfoForRow(
return new long[] {memTableIncrement, textDataIncrement, chunkMetadataIncrement};
}
- @SuppressWarnings("squid:S3776") // High Cognitive Complexity
private long[] checkMemCostAndAddToTspInfoForRows(InsertRowsNode insertRowsNode)
throws WriteProcessException {
- // Memory of increased PrimitiveArray and TEXT values, e.g., add a long[128], add 128*8
- long memTableIncrement = 0L;
- long textDataIncrement = 0L;
- long chunkMetadataIncrement = 0L;
+
+ long[] memIncrements = new long[3];
// device -> measurement -> adding TVList size
- Map<IDeviceID, Map<String, Integer>> increasingMemTableInfo = new HashMap<>();
+ Map<IDeviceID, Map<String, Integer>> increasingMemTableInfoForNonAligned = new HashMap<>();
+ // device -> (measurements -> datatype, adding aligned TVList size)
+ Map<IDeviceID, Pair<Map<String, TSDataType>, Integer>> increasingMemTableInfoForAligned =
+ new HashMap<>();
for (InsertRowNode insertRowNode : insertRowsNode.getInsertRowNodeList()) {
- IDeviceID deviceId = insertRowNode.getDeviceID();
- TSDataType[] dataTypes = insertRowNode.getDataTypes();
- Object[] values = insertRowNode.getValues();
- String[] measurements = insertRowNode.getMeasurements();
+ if (insertRowNode.isAligned()) {
+ handleAlignedData(insertRowNode, memIncrements, increasingMemTableInfoForAligned);
+ } else {
+ handleUnalignedData(insertRowNode, memIncrements, increasingMemTableInfoForNonAligned);
+ }
+ }
+ updateMemoryInfo(memIncrements[0], memIncrements[2], memIncrements[1]);
Review Comment:
Seeing this, I would recommend wrapping them with a simple class. It may be easy for other people to confuse or make a typo.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/memtable/TsFileProcessor.java:
##########
@@ -561,55 +557,163 @@ private long[] checkMemCostAndAddToTspInfoForRow(
return new long[] {memTableIncrement, textDataIncrement, chunkMetadataIncrement};
}
- @SuppressWarnings("squid:S3776") // High Cognitive Complexity
private long[] checkMemCostAndAddToTspInfoForRows(InsertRowsNode insertRowsNode)
throws WriteProcessException {
- // Memory of increased PrimitiveArray and TEXT values, e.g., add a long[128], add 128*8
- long memTableIncrement = 0L;
- long textDataIncrement = 0L;
- long chunkMetadataIncrement = 0L;
+
+ long[] memIncrements = new long[3];
// device -> measurement -> adding TVList size
- Map<IDeviceID, Map<String, Integer>> increasingMemTableInfo = new HashMap<>();
+ Map<IDeviceID, Map<String, Integer>> increasingMemTableInfoForNonAligned = new HashMap<>();
+ // device -> (measurements -> datatype, adding aligned TVList size)
+ Map<IDeviceID, Pair<Map<String, TSDataType>, Integer>> increasingMemTableInfoForAligned =
+ new HashMap<>();
for (InsertRowNode insertRowNode : insertRowsNode.getInsertRowNodeList()) {
- IDeviceID deviceId = insertRowNode.getDeviceID();
- TSDataType[] dataTypes = insertRowNode.getDataTypes();
- Object[] values = insertRowNode.getValues();
- String[] measurements = insertRowNode.getMeasurements();
+ if (insertRowNode.isAligned()) {
+ handleAlignedData(insertRowNode, memIncrements, increasingMemTableInfoForAligned);
+ } else {
+ handleUnalignedData(insertRowNode, memIncrements, increasingMemTableInfoForNonAligned);
+ }
+ }
+ updateMemoryInfo(memIncrements[0], memIncrements[2], memIncrements[1]);
+ return memIncrements;
+ }
+
+ @SuppressWarnings("squid:S3776") // High Cognitive Complexity
+ private void handleAlignedData(
+ InsertRowNode insertRowNode,
+ long[] memIncrements,
+ Map<IDeviceID, Pair<Map<String, TSDataType>, Integer>> increasingMemTableInfoForAligned) {
+ long memTableIncrement = memIncrements[0];
+ long textDataIncrement = memIncrements[1];
+ long chunkMetadataIncrement = memIncrements[2];
+
+ IDeviceID deviceId = insertRowNode.getDeviceID();
+ TSDataType[] dataTypes = insertRowNode.getDataTypes();
+ Object[] values = insertRowNode.getValues();
+ String[] measurements = insertRowNode.getMeasurements();
+
+ if (workMemTable.checkIfChunkDoesNotExist(deviceId, AlignedPath.VECTOR_PLACEHOLDER)
+ && !increasingMemTableInfoForAligned.containsKey(deviceId)) {
+ // For new device of this mem table
+ // ChunkMetadataIncrement
+ chunkMetadataIncrement +=
+ ChunkMetadata.calculateRamSize(AlignedPath.VECTOR_PLACEHOLDER, TSDataType.VECTOR)
+ * dataTypes.length;
+ memTableIncrement += AlignedTVList.alignedTvListArrayMemCost(dataTypes);
for (int i = 0; i < dataTypes.length; i++) {
// Skip failed Measurements
if (dataTypes[i] == null || measurements[i] == null) {
continue;
}
- if (workMemTable.checkIfChunkDoesNotExist(deviceId, measurements[i])
- && (!increasingMemTableInfo.containsKey(deviceId)
- || !increasingMemTableInfo.get(deviceId).containsKey(measurements[i]))) {
- // ChunkMetadataIncrement
- chunkMetadataIncrement += ChunkMetadata.calculateRamSize(measurements[i], dataTypes[i]);
- memTableIncrement += TVList.tvListArrayMemCost(dataTypes[i]);
- increasingMemTableInfo
- .computeIfAbsent(deviceId, k -> new HashMap<>())
- .putIfAbsent(measurements[i], 1);
- } else {
- // here currentChunkPointNum >= 1
- long currentChunkPointNum = workMemTable.getCurrentTVListSize(deviceId, measurements[i]);
- int addingPointNum =
- increasingMemTableInfo
- .computeIfAbsent(deviceId, k -> new HashMap<>())
- .computeIfAbsent(measurements[i], k -> 0);
+ increasingMemTableInfoForAligned
+ .computeIfAbsent(deviceId, k -> new Pair<>(new HashMap<>(), 1))
+ .left
+ .put(measurements[i], dataTypes[i]);
+ // TEXT data mem size
+ if (dataTypes[i] == TSDataType.TEXT && values[i] != null) {
+ textDataIncrement += MemUtils.getBinarySize((Binary) values[i]);
+ }
+ }
+
+ } else {
+ // For existed device of this mem table
+ AlignedWritableMemChunkGroup memChunkGroup =
+ (AlignedWritableMemChunkGroup) workMemTable.getMemTableMap().get(deviceId);
+ AlignedWritableMemChunk alignedMemChunk =
+ memChunkGroup == null ? null : memChunkGroup.getAlignedMemChunk();
+ long currentChunkPointNum = alignedMemChunk == null ? 0 : alignedMemChunk.alignedListSize();
+ List<TSDataType> dataTypesInTVList = new ArrayList<>();
+ Pair<Map<String, TSDataType>, Integer> addingPointNumInfo =
+ increasingMemTableInfoForAligned.computeIfAbsent(
+ deviceId, k -> new Pair<>(new HashMap<>(), 0));
+ for (int i = 0; i < dataTypes.length; i++) {
+ // Skip failed Measurements
+ if (dataTypes[i] == null || measurements[i] == null) {
+ continue;
+ }
+
+ int addingPointNum = addingPointNumInfo.getRight();
+ // Extending the column of aligned mem chunk
+ if ((alignedMemChunk != null && !alignedMemChunk.containsMeasurement(measurements[i]))
+ && !increasingMemTableInfoForAligned.get(deviceId).left.containsKey(measurements[i])) {
memTableIncrement +=
- ((currentChunkPointNum + addingPointNum) % PrimitiveArrayManager.ARRAY_SIZE) == 0
- ? TVList.tvListArrayMemCost(dataTypes[i])
- : 0;
- increasingMemTableInfo.get(deviceId).computeIfPresent(measurements[i], (k, v) -> v + 1);
+ ((currentChunkPointNum + addingPointNum) / PrimitiveArrayManager.ARRAY_SIZE + 1)
+ * AlignedTVList.valueListArrayMemCost(dataTypes[i]);
+ increasingMemTableInfoForAligned.get(deviceId).left.put(measurements[i], dataTypes[i]);
}
// TEXT data mem size
- if (dataTypes[i].isBinary() && values[i] != null) {
+ if (dataTypes[i] == TSDataType.TEXT && values[i] != null) {
Review Comment:
How about BINARY and BLOB?
##########
iotdb-core/datanode/src/test/java/org/apache/iotdb/db/storageengine/dataregion/memtable/TsFileProcessorTest.java:
##########
@@ -613,6 +613,65 @@ record = new TSRecord(102, "root.vehicle.d2");
Assert.assertEquals(memTable1.memSize(), memTable2.memSize());
}
+ @Test
+ public void testRamCostInsertSameDataBy2Ways()
Review Comment:
Better to add some tests against the corner cases I mentioned above (if they are real).
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix memory calculate error when insertRecords with both aligned and non-aligned devices [iotdb]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #12720:
URL: https://github.com/apache/iotdb/pull/12720#issuecomment-2348672269
## [](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=12720) **Quality Gate passed**
Issues
 [11 New issues](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=12720&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=12720&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_iotdb&pullRequest=12720&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=12720&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=12720&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=12720)
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix memory calculate error when insertRecords with both aligned and non-aligned devices [iotdb]
Posted by "HTHou (via GitHub)" <gi...@apache.org>.
HTHou merged PR #12720:
URL: https://github.com/apache/iotdb/pull/12720
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix memory calculate error when insertRecords with both aligned and non-aligned devices [iotdb]
Posted by "jt2594838 (via GitHub)" <gi...@apache.org>.
jt2594838 commented on code in PR #12720:
URL: https://github.com/apache/iotdb/pull/12720#discussion_r1639355843
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/memtable/TsFileProcessor.java:
##########
@@ -561,55 +557,163 @@ private long[] checkMemCostAndAddToTspInfoForRow(
return new long[] {memTableIncrement, textDataIncrement, chunkMetadataIncrement};
}
- @SuppressWarnings("squid:S3776") // High Cognitive Complexity
private long[] checkMemCostAndAddToTspInfoForRows(InsertRowsNode insertRowsNode)
throws WriteProcessException {
- // Memory of increased PrimitiveArray and TEXT values, e.g., add a long[128], add 128*8
- long memTableIncrement = 0L;
- long textDataIncrement = 0L;
- long chunkMetadataIncrement = 0L;
+
+ long[] memIncrements = new long[3];
// device -> measurement -> adding TVList size
- Map<IDeviceID, Map<String, Integer>> increasingMemTableInfo = new HashMap<>();
+ Map<IDeviceID, Map<String, Integer>> increasingMemTableInfoForNonAligned = new HashMap<>();
+ // device -> (measurements -> datatype, adding aligned TVList size)
+ Map<IDeviceID, Pair<Map<String, TSDataType>, Integer>> increasingMemTableInfoForAligned =
+ new HashMap<>();
for (InsertRowNode insertRowNode : insertRowsNode.getInsertRowNodeList()) {
- IDeviceID deviceId = insertRowNode.getDeviceID();
- TSDataType[] dataTypes = insertRowNode.getDataTypes();
- Object[] values = insertRowNode.getValues();
- String[] measurements = insertRowNode.getMeasurements();
+ if (insertRowNode.isAligned()) {
+ handleAlignedData(insertRowNode, memIncrements, increasingMemTableInfoForAligned);
+ } else {
+ handleUnalignedData(insertRowNode, memIncrements, increasingMemTableInfoForNonAligned);
+ }
+ }
+ updateMemoryInfo(memIncrements[0], memIncrements[2], memIncrements[1]);
+ return memIncrements;
+ }
+
+ @SuppressWarnings("squid:S3776") // High Cognitive Complexity
+ private void handleAlignedData(
+ InsertRowNode insertRowNode,
+ long[] memIncrements,
+ Map<IDeviceID, Pair<Map<String, TSDataType>, Integer>> increasingMemTableInfoForAligned) {
+ long memTableIncrement = memIncrements[0];
+ long textDataIncrement = memIncrements[1];
+ long chunkMetadataIncrement = memIncrements[2];
+
+ IDeviceID deviceId = insertRowNode.getDeviceID();
+ TSDataType[] dataTypes = insertRowNode.getDataTypes();
+ Object[] values = insertRowNode.getValues();
+ String[] measurements = insertRowNode.getMeasurements();
+
+ if (workMemTable.checkIfChunkDoesNotExist(deviceId, AlignedPath.VECTOR_PLACEHOLDER)
+ && !increasingMemTableInfoForAligned.containsKey(deviceId)) {
+ // For new device of this mem table
+ // ChunkMetadataIncrement
+ chunkMetadataIncrement +=
+ ChunkMetadata.calculateRamSize(AlignedPath.VECTOR_PLACEHOLDER, TSDataType.VECTOR)
+ * dataTypes.length;
+ memTableIncrement += AlignedTVList.alignedTvListArrayMemCost(dataTypes);
for (int i = 0; i < dataTypes.length; i++) {
// Skip failed Measurements
if (dataTypes[i] == null || measurements[i] == null) {
continue;
}
- if (workMemTable.checkIfChunkDoesNotExist(deviceId, measurements[i])
- && (!increasingMemTableInfo.containsKey(deviceId)
- || !increasingMemTableInfo.get(deviceId).containsKey(measurements[i]))) {
- // ChunkMetadataIncrement
- chunkMetadataIncrement += ChunkMetadata.calculateRamSize(measurements[i], dataTypes[i]);
- memTableIncrement += TVList.tvListArrayMemCost(dataTypes[i]);
- increasingMemTableInfo
- .computeIfAbsent(deviceId, k -> new HashMap<>())
- .putIfAbsent(measurements[i], 1);
- } else {
- // here currentChunkPointNum >= 1
- long currentChunkPointNum = workMemTable.getCurrentTVListSize(deviceId, measurements[i]);
- int addingPointNum =
- increasingMemTableInfo
- .computeIfAbsent(deviceId, k -> new HashMap<>())
- .computeIfAbsent(measurements[i], k -> 0);
+ increasingMemTableInfoForAligned
+ .computeIfAbsent(deviceId, k -> new Pair<>(new HashMap<>(), 1))
+ .left
+ .put(measurements[i], dataTypes[i]);
+ // TEXT data mem size
+ if (dataTypes[i] == TSDataType.TEXT && values[i] != null) {
+ textDataIncrement += MemUtils.getBinarySize((Binary) values[i]);
+ }
+ }
+
+ } else {
+ // For existed device of this mem table
+ AlignedWritableMemChunkGroup memChunkGroup =
+ (AlignedWritableMemChunkGroup) workMemTable.getMemTableMap().get(deviceId);
+ AlignedWritableMemChunk alignedMemChunk =
+ memChunkGroup == null ? null : memChunkGroup.getAlignedMemChunk();
+ long currentChunkPointNum = alignedMemChunk == null ? 0 : alignedMemChunk.alignedListSize();
+ List<TSDataType> dataTypesInTVList = new ArrayList<>();
+ Pair<Map<String, TSDataType>, Integer> addingPointNumInfo =
+ increasingMemTableInfoForAligned.computeIfAbsent(
+ deviceId, k -> new Pair<>(new HashMap<>(), 0));
+ for (int i = 0; i < dataTypes.length; i++) {
+ // Skip failed Measurements
+ if (dataTypes[i] == null || measurements[i] == null) {
+ continue;
+ }
+
+ int addingPointNum = addingPointNumInfo.getRight();
+ // Extending the column of aligned mem chunk
+ if ((alignedMemChunk != null && !alignedMemChunk.containsMeasurement(measurements[i]))
+ && !increasingMemTableInfoForAligned.get(deviceId).left.containsKey(measurements[i])) {
memTableIncrement +=
- ((currentChunkPointNum + addingPointNum) % PrimitiveArrayManager.ARRAY_SIZE) == 0
- ? TVList.tvListArrayMemCost(dataTypes[i])
- : 0;
- increasingMemTableInfo.get(deviceId).computeIfPresent(measurements[i], (k, v) -> v + 1);
+ ((currentChunkPointNum + addingPointNum) / PrimitiveArrayManager.ARRAY_SIZE + 1)
+ * AlignedTVList.valueListArrayMemCost(dataTypes[i]);
Review Comment:
This does not seem like "increment".
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix memory calculate error when insertRecords with both aligned and non-aligned devices [iotdb]
Posted by "HTHou (via GitHub)" <gi...@apache.org>.
HTHou commented on code in PR #12720:
URL: https://github.com/apache/iotdb/pull/12720#discussion_r1758667757
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/memtable/TsFileProcessor.java:
##########
@@ -561,55 +557,163 @@ private long[] checkMemCostAndAddToTspInfoForRow(
return new long[] {memTableIncrement, textDataIncrement, chunkMetadataIncrement};
}
- @SuppressWarnings("squid:S3776") // High Cognitive Complexity
private long[] checkMemCostAndAddToTspInfoForRows(InsertRowsNode insertRowsNode)
throws WriteProcessException {
- // Memory of increased PrimitiveArray and TEXT values, e.g., add a long[128], add 128*8
- long memTableIncrement = 0L;
- long textDataIncrement = 0L;
- long chunkMetadataIncrement = 0L;
+
+ long[] memIncrements = new long[3];
// device -> measurement -> adding TVList size
- Map<IDeviceID, Map<String, Integer>> increasingMemTableInfo = new HashMap<>();
+ Map<IDeviceID, Map<String, Integer>> increasingMemTableInfoForNonAligned = new HashMap<>();
+ // device -> (measurements -> datatype, adding aligned TVList size)
+ Map<IDeviceID, Pair<Map<String, TSDataType>, Integer>> increasingMemTableInfoForAligned =
+ new HashMap<>();
for (InsertRowNode insertRowNode : insertRowsNode.getInsertRowNodeList()) {
- IDeviceID deviceId = insertRowNode.getDeviceID();
- TSDataType[] dataTypes = insertRowNode.getDataTypes();
- Object[] values = insertRowNode.getValues();
- String[] measurements = insertRowNode.getMeasurements();
+ if (insertRowNode.isAligned()) {
+ handleAlignedData(insertRowNode, memIncrements, increasingMemTableInfoForAligned);
+ } else {
+ handleUnalignedData(insertRowNode, memIncrements, increasingMemTableInfoForNonAligned);
+ }
+ }
+ updateMemoryInfo(memIncrements[0], memIncrements[2], memIncrements[1]);
+ return memIncrements;
+ }
+
+ @SuppressWarnings("squid:S3776") // High Cognitive Complexity
+ private void handleAlignedData(
Review Comment:
fixed
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix memory calculate error when insertRecords with both aligned and non-aligned devices [iotdb]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #12720:
URL: https://github.com/apache/iotdb/pull/12720#issuecomment-2348687377
## [](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=12720) **Quality Gate passed**
Issues
 [11 New issues](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=12720&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=12720&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_iotdb&pullRequest=12720&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=12720&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=12720&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=12720)
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix memory calculate error when insertRecords with both aligned and non-aligned devices [iotdb]
Posted by "HTHou (via GitHub)" <gi...@apache.org>.
HTHou commented on code in PR #12720:
URL: https://github.com/apache/iotdb/pull/12720#discussion_r1758665470
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/memtable/TsFileProcessor.java:
##########
@@ -561,55 +557,163 @@ private long[] checkMemCostAndAddToTspInfoForRow(
return new long[] {memTableIncrement, textDataIncrement, chunkMetadataIncrement};
}
- @SuppressWarnings("squid:S3776") // High Cognitive Complexity
private long[] checkMemCostAndAddToTspInfoForRows(InsertRowsNode insertRowsNode)
throws WriteProcessException {
- // Memory of increased PrimitiveArray and TEXT values, e.g., add a long[128], add 128*8
- long memTableIncrement = 0L;
- long textDataIncrement = 0L;
- long chunkMetadataIncrement = 0L;
+
+ long[] memIncrements = new long[3];
// device -> measurement -> adding TVList size
- Map<IDeviceID, Map<String, Integer>> increasingMemTableInfo = new HashMap<>();
+ Map<IDeviceID, Map<String, Integer>> increasingMemTableInfoForNonAligned = new HashMap<>();
+ // device -> (measurements -> datatype, adding aligned TVList size)
+ Map<IDeviceID, Pair<Map<String, TSDataType>, Integer>> increasingMemTableInfoForAligned =
+ new HashMap<>();
for (InsertRowNode insertRowNode : insertRowsNode.getInsertRowNodeList()) {
- IDeviceID deviceId = insertRowNode.getDeviceID();
- TSDataType[] dataTypes = insertRowNode.getDataTypes();
- Object[] values = insertRowNode.getValues();
- String[] measurements = insertRowNode.getMeasurements();
+ if (insertRowNode.isAligned()) {
+ handleAlignedData(insertRowNode, memIncrements, increasingMemTableInfoForAligned);
+ } else {
+ handleUnalignedData(insertRowNode, memIncrements, increasingMemTableInfoForNonAligned);
+ }
+ }
+ updateMemoryInfo(memIncrements[0], memIncrements[2], memIncrements[1]);
+ return memIncrements;
+ }
+
+ @SuppressWarnings("squid:S3776") // High Cognitive Complexity
+ private void handleAlignedData(
+ InsertRowNode insertRowNode,
+ long[] memIncrements,
+ Map<IDeviceID, Pair<Map<String, TSDataType>, Integer>> increasingMemTableInfoForAligned) {
+ long memTableIncrement = memIncrements[0];
+ long textDataIncrement = memIncrements[1];
+ long chunkMetadataIncrement = memIncrements[2];
+
+ IDeviceID deviceId = insertRowNode.getDeviceID();
+ TSDataType[] dataTypes = insertRowNode.getDataTypes();
+ Object[] values = insertRowNode.getValues();
+ String[] measurements = insertRowNode.getMeasurements();
+
+ if (workMemTable.checkIfChunkDoesNotExist(deviceId, AlignedPath.VECTOR_PLACEHOLDER)
+ && !increasingMemTableInfoForAligned.containsKey(deviceId)) {
+ // For new device of this mem table
+ // ChunkMetadataIncrement
+ chunkMetadataIncrement +=
+ ChunkMetadata.calculateRamSize(AlignedPath.VECTOR_PLACEHOLDER, TSDataType.VECTOR)
+ * dataTypes.length;
+ memTableIncrement += AlignedTVList.alignedTvListArrayMemCost(dataTypes);
for (int i = 0; i < dataTypes.length; i++) {
// Skip failed Measurements
if (dataTypes[i] == null || measurements[i] == null) {
continue;
}
- if (workMemTable.checkIfChunkDoesNotExist(deviceId, measurements[i])
- && (!increasingMemTableInfo.containsKey(deviceId)
- || !increasingMemTableInfo.get(deviceId).containsKey(measurements[i]))) {
- // ChunkMetadataIncrement
- chunkMetadataIncrement += ChunkMetadata.calculateRamSize(measurements[i], dataTypes[i]);
- memTableIncrement += TVList.tvListArrayMemCost(dataTypes[i]);
- increasingMemTableInfo
- .computeIfAbsent(deviceId, k -> new HashMap<>())
- .putIfAbsent(measurements[i], 1);
- } else {
- // here currentChunkPointNum >= 1
- long currentChunkPointNum = workMemTable.getCurrentTVListSize(deviceId, measurements[i]);
- int addingPointNum =
- increasingMemTableInfo
- .computeIfAbsent(deviceId, k -> new HashMap<>())
- .computeIfAbsent(measurements[i], k -> 0);
+ increasingMemTableInfoForAligned
+ .computeIfAbsent(deviceId, k -> new Pair<>(new HashMap<>(), 1))
+ .left
+ .put(measurements[i], dataTypes[i]);
+ // TEXT data mem size
+ if (dataTypes[i] == TSDataType.TEXT && values[i] != null) {
+ textDataIncrement += MemUtils.getBinarySize((Binary) values[i]);
+ }
+ }
+
+ } else {
+ // For existed device of this mem table
+ AlignedWritableMemChunkGroup memChunkGroup =
+ (AlignedWritableMemChunkGroup) workMemTable.getMemTableMap().get(deviceId);
+ AlignedWritableMemChunk alignedMemChunk =
+ memChunkGroup == null ? null : memChunkGroup.getAlignedMemChunk();
+ long currentChunkPointNum = alignedMemChunk == null ? 0 : alignedMemChunk.alignedListSize();
+ List<TSDataType> dataTypesInTVList = new ArrayList<>();
+ Pair<Map<String, TSDataType>, Integer> addingPointNumInfo =
+ increasingMemTableInfoForAligned.computeIfAbsent(
+ deviceId, k -> new Pair<>(new HashMap<>(), 0));
+ for (int i = 0; i < dataTypes.length; i++) {
+ // Skip failed Measurements
+ if (dataTypes[i] == null || measurements[i] == null) {
+ continue;
+ }
+
+ int addingPointNum = addingPointNumInfo.getRight();
+ // Extending the column of aligned mem chunk
+ if ((alignedMemChunk != null && !alignedMemChunk.containsMeasurement(measurements[i]))
+ && !increasingMemTableInfoForAligned.get(deviceId).left.containsKey(measurements[i])) {
memTableIncrement +=
- ((currentChunkPointNum + addingPointNum) % PrimitiveArrayManager.ARRAY_SIZE) == 0
- ? TVList.tvListArrayMemCost(dataTypes[i])
- : 0;
- increasingMemTableInfo.get(deviceId).computeIfPresent(measurements[i], (k, v) -> v + 1);
+ ((currentChunkPointNum + addingPointNum) / PrimitiveArrayManager.ARRAY_SIZE + 1)
+ * AlignedTVList.valueListArrayMemCost(dataTypes[i]);
+ increasingMemTableInfoForAligned.get(deviceId).left.put(measurements[i], dataTypes[i]);
}
// TEXT data mem size
- if (dataTypes[i].isBinary() && values[i] != null) {
+ if (dataTypes[i] == TSDataType.TEXT && values[i] != null) {
Review Comment:
Fixed.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix memory calculate error when insertRecords with both aligned and non-aligned devices [iotdb]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #12720:
URL: https://github.com/apache/iotdb/pull/12720#issuecomment-2350767918
## [](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=12720) **Quality Gate passed**
Issues
 [20 New issues](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=12720&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=12720&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_iotdb&pullRequest=12720&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=12720&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=12720&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=12720)
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix memory calculate error when insertRecords with both aligned and non-aligned devices [iotdb]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #12720:
URL: https://github.com/apache/iotdb/pull/12720#issuecomment-2350787904
## [](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=12720) **Quality Gate failed**
Failed conditions
 [3.4% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=12720&metric=new_duplicated_lines_density&view=list) (required ≤ 3%)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=12720)
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org