You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "Caideyipi (via GitHub)" <gi...@apache.org> on 2026/06/25 04:46:29 UTC
[PR] Fix tag index memory accounting [iotdb]
Caideyipi opened a new pull request, #18029:
URL: https://github.com/apache/iotdb/pull/18029
## Description
This PR makes TagManager update tag index memory accounting based on actual map/set mutations instead of non-atomic size snapshots.
It also makes index removal tolerant of missing entries and uses a concurrent measurement set for tag-index readers.
## Tests
- mvn spotless:apply -pl iotdb-core/datanode passed before the final one-line concurrent-set adjustment.
- git diff --check passed.
- mvn -pl iotdb-core/datanode -Dtest=TagManagerTest test could not reach the test phase locally because datanode compilation failed on unrelated generated/query/pipe classes, e.g. missing AGGREGATION_OPERATOR_FROM_RAW_DATA, Accumulator/IFill, and pipe inclusion constants from dependent modules/classes.
- A later mvn spotless:apply -pl iotdb-core/datanode retry failed to start the JVM due to local Windows page-file/native-memory exhaustion (DOS error/errno=1455).
--
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 tag index memory accounting [iotdb]
Posted by "Caideyipi (via GitHub)" <gi...@apache.org>.
Caideyipi commented on code in PR #18029:
URL: https://github.com/apache/iotdb/pull/18029#discussion_r3472954764
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/schemaregion/tag/TagManager.java:
##########
@@ -166,34 +165,33 @@ public void addIndex(String tagKey, String tagValue, IMeasurementMNode<?> measur
return;
}
- int tagIndexOldSize = tagIndex.size();
- Map<String, Set<IMeasurementMNode<?>>> tagValueMap =
- tagIndex.computeIfAbsent(tagKey, k -> new ConcurrentHashMap<>());
- int tagIndexNewSize = tagIndex.size();
-
- int tagValueMapOldSize = tagValueMap.size();
- Set<IMeasurementMNode<?>> measurementsSet =
- tagValueMap.computeIfAbsent(tagValue, v -> Collections.synchronizedSet(new HashSet<>()));
- int tagValueMapNewSize = tagValueMap.size();
+ tagIndex.compute(
+ tagKey,
+ (key, tagValueMap) -> {
+ long memorySize = 0;
+ if (tagValueMap == null) {
+ tagValueMap = new ConcurrentHashMap<>();
+ // the last 4 is the memory occupied by the size of tagvaluemap
+ memorySize += RamUsageEstimator.sizeOf(tagKey) + 4;
Review Comment:
Followed up in 28a97160245: renamed the constant to INDEX_ENTRY_OVERHEAD_ESTIMATE_BYTES and added a comment clarifying that it is an int-sized accounting estimate for each indexed key/value/measurement reference, not a specific ConcurrentHashMap or Set field.
--
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 tag index memory accounting [iotdb]
Posted by "jt2594838 (via GitHub)" <gi...@apache.org>.
jt2594838 merged PR #18029:
URL: https://github.com/apache/iotdb/pull/18029
--
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 tag index memory accounting [iotdb]
Posted by "Caideyipi (via GitHub)" <gi...@apache.org>.
Caideyipi commented on code in PR #18029:
URL: https://github.com/apache/iotdb/pull/18029#discussion_r3472891169
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/schemaregion/tag/TagManager.java:
##########
@@ -166,34 +165,33 @@ public void addIndex(String tagKey, String tagValue, IMeasurementMNode<?> measur
return;
}
- int tagIndexOldSize = tagIndex.size();
- Map<String, Set<IMeasurementMNode<?>>> tagValueMap =
- tagIndex.computeIfAbsent(tagKey, k -> new ConcurrentHashMap<>());
- int tagIndexNewSize = tagIndex.size();
-
- int tagValueMapOldSize = tagValueMap.size();
- Set<IMeasurementMNode<?>> measurementsSet =
- tagValueMap.computeIfAbsent(tagValue, v -> Collections.synchronizedSet(new HashSet<>()));
- int tagValueMapNewSize = tagValueMap.size();
+ tagIndex.compute(
+ tagKey,
+ (key, tagValueMap) -> {
+ long memorySize = 0;
+ if (tagValueMap == null) {
+ tagValueMap = new ConcurrentHashMap<>();
+ // the last 4 is the memory occupied by the size of tagvaluemap
+ memorySize += RamUsageEstimator.sizeOf(tagKey) + 4;
Review Comment:
Good point. I replaced the hard-coded 4 with COLLECTION_SIZE_FIELD_BYTES = Integer.BYTES for the estimated collection size field and removed the misleading comments in ee56b901e8c.
--
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 tag index memory accounting [iotdb]
Posted by "jt2594838 (via GitHub)" <gi...@apache.org>.
jt2594838 commented on code in PR #18029:
URL: https://github.com/apache/iotdb/pull/18029#discussion_r3472592792
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/schemaregion/tag/TagManager.java:
##########
@@ -166,34 +165,33 @@ public void addIndex(String tagKey, String tagValue, IMeasurementMNode<?> measur
return;
}
- int tagIndexOldSize = tagIndex.size();
- Map<String, Set<IMeasurementMNode<?>>> tagValueMap =
- tagIndex.computeIfAbsent(tagKey, k -> new ConcurrentHashMap<>());
- int tagIndexNewSize = tagIndex.size();
-
- int tagValueMapOldSize = tagValueMap.size();
- Set<IMeasurementMNode<?>> measurementsSet =
- tagValueMap.computeIfAbsent(tagValue, v -> Collections.synchronizedSet(new HashSet<>()));
- int tagValueMapNewSize = tagValueMap.size();
+ tagIndex.compute(
+ tagKey,
+ (key, tagValueMap) -> {
+ long memorySize = 0;
+ if (tagValueMap == null) {
+ tagValueMap = new ConcurrentHashMap<>();
+ // the last 4 is the memory occupied by the size of tagvaluemap
+ memorySize += RamUsageEstimator.sizeOf(tagKey) + 4;
Review Comment:
Why is it only 4 bytes? The same below
--
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