Skip to content

Commit 3259b32

Browse files
CaideyipirobertMileaNi
authored andcommitted
Fix tag index memory accounting (#18029)
* Fix tag index memory accounting * Add concurrent tag index memory test * Clarify tag index size accounting * Clarify tag index overhead estimate * Fix pipe source time range IT flakiness
1 parent 00501c2 commit 3259b32

3 files changed

Lines changed: 309 additions & 64 deletions

File tree

integration-test/src/test/java/org/apache/iotdb/pipe/it/dual/treemodel/auto/basic/IoTDBPipeSourceIT.java

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
import java.util.HashSet;
5656
import java.util.List;
5757
import java.util.Map;
58+
import java.util.function.Consumer;
5859

5960
import static org.junit.Assert.fail;
6061

@@ -778,6 +779,11 @@ public void testExtractorTimeRangeMatch() throws Exception {
778779

779780
final String receiverIp = receiverDataNode.getIp();
780781
final int receiverPort = receiverDataNode.getPort();
782+
final Consumer<String> handleFailure =
783+
o -> {
784+
TestUtils.executeNonQueryWithRetry(senderEnv, "flush");
785+
TestUtils.executeNonQueryWithRetry(receiverEnv, "flush");
786+
};
781787

782788
try (final SyncConfigNodeIServiceClient client =
783789
(SyncConfigNodeIServiceClient) senderEnv.getLeaderConfigNodeConnection()) {
@@ -814,10 +820,12 @@ public void testExtractorTimeRangeMatch() throws Exception {
814820
.setProcessorAttributes(processorAttributes));
815821
Assert.assertEquals(TSStatusCode.SUCCESS_STATUS.getStatusCode(), status.getCode());
816822

817-
final Map<String, String> expectedCountResult = new HashMap<>();
818-
expectedCountResult.put("count(root.db.d1.at1)", "3");
819823
TestUtils.assertDataEventuallyOnEnv(
820-
receiverEnv, "select count(*) from root.db.**", expectedCountResult);
824+
receiverEnv,
825+
"select count(at1) from root.db.d1",
826+
"count(root.db.d1.at1),",
827+
Collections.singleton("3,"),
828+
handleFailure);
821829

822830
// Insert realtime data that overlapped with time range
823831
TestUtils.executeNonQueries(
@@ -828,9 +836,12 @@ public void testExtractorTimeRangeMatch() throws Exception {
828836
"flush"),
829837
null);
830838

831-
expectedCountResult.put("count(root.db.d3.at1)", "3");
832839
TestUtils.assertDataEventuallyOnEnv(
833-
receiverEnv, "select count(*) from root.db.**", expectedCountResult);
840+
receiverEnv,
841+
"select count(at1) from root.db.d1, root.db.d3",
842+
"count(root.db.d1.at1),count(root.db.d3.at1),",
843+
Collections.singleton("3,3,"),
844+
handleFailure);
834845

835846
// Session Tablet can have unused timestamp slots when rowSize is smaller than maxRowNumber.
836847
// The pipe source time range filter should ignore the unused zero tail.
@@ -848,9 +859,12 @@ public void testExtractorTimeRangeMatch() throws Exception {
848859
session.insertTablet(tabletWithUnusedTail);
849860
}
850861

851-
expectedCountResult.put("count(root.db.d5.at1)", "3");
852862
TestUtils.assertDataEventuallyOnEnv(
853-
receiverEnv, "select count(*) from root.db.**", expectedCountResult);
863+
receiverEnv,
864+
"select count(at1) from root.db.d1, root.db.d3, root.db.d5",
865+
"count(root.db.d1.at1),count(root.db.d3.at1),count(root.db.d5.at1),",
866+
Collections.singleton("3,3,3,"),
867+
handleFailure);
854868

855869
// Insert realtime data that does not overlap with time range
856870
TestUtils.executeNonQueries(
@@ -865,7 +879,9 @@ public void testExtractorTimeRangeMatch() throws Exception {
865879
receiverEnv,
866880
"select count(at1) from root.db.d1, root.db.d3, root.db.d5",
867881
"count(root.db.d1.at1),count(root.db.d3.at1),count(root.db.d5.at1),",
868-
Collections.singleton("3,3,3,"));
882+
Collections.singleton("3,3,3,"),
883+
600,
884+
handleFailure);
869885
TestUtils.assertDataAlwaysOnEnv(
870886
receiverEnv,
871887
"show timeseries root.db.d2.**",

iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/schemaregion/tag/TagManager.java

Lines changed: 69 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@
5252
import java.util.Collections;
5353
import java.util.Comparator;
5454
import java.util.HashMap;
55-
import java.util.HashSet;
5655
import java.util.Iterator;
5756
import java.util.List;
5857
import java.util.Map;
@@ -71,6 +70,10 @@ public class TagManager {
7170
private static final String PREVIOUS_CONDITION =
7271
"before deleting it, tag key is %s, tag value is %s, tlog offset is %d, contains key %b";
7372

73+
// The tag index memory model adds one int-sized estimated overhead for each indexed key, value,
74+
// and measurement reference. This is an accounting estimate rather than a specific
75+
// ConcurrentHashMap or Set field.
76+
private static final long INDEX_ENTRY_OVERHEAD_ESTIMATE_BYTES = Integer.BYTES;
7477
private static final Logger logger = LoggerFactory.getLogger(TagManager.class);
7578
private static final CommonConfig COMMON_CONFIG = CommonDescriptor.getInstance().getConfig();
7679

@@ -166,34 +169,31 @@ public void addIndex(String tagKey, String tagValue, IMeasurementMNode<?> measur
166169
return;
167170
}
168171

169-
int tagIndexOldSize = tagIndex.size();
170-
Map<String, Set<IMeasurementMNode<?>>> tagValueMap =
171-
tagIndex.computeIfAbsent(tagKey, k -> new ConcurrentHashMap<>());
172-
int tagIndexNewSize = tagIndex.size();
173-
174-
int tagValueMapOldSize = tagValueMap.size();
175-
Set<IMeasurementMNode<?>> measurementsSet =
176-
tagValueMap.computeIfAbsent(tagValue, v -> Collections.synchronizedSet(new HashSet<>()));
177-
int tagValueMapNewSize = tagValueMap.size();
172+
tagIndex.compute(
173+
tagKey,
174+
(key, tagValueMap) -> {
175+
long memorySize = 0;
176+
if (tagValueMap == null) {
177+
tagValueMap = new ConcurrentHashMap<>();
178+
memorySize += RamUsageEstimator.sizeOf(tagKey) + INDEX_ENTRY_OVERHEAD_ESTIMATE_BYTES;
179+
}
178180

179-
int measurementsSetOldSize = measurementsSet.size();
180-
measurementsSet.add(measurementMNode);
181-
int measurementsSetNewSize = measurementsSet.size();
181+
Set<IMeasurementMNode<?>> measurementsSet = tagValueMap.get(tagValue);
182+
if (measurementsSet == null) {
183+
measurementsSet = ConcurrentHashMap.newKeySet();
184+
tagValueMap.put(tagValue, measurementsSet);
185+
memorySize += RamUsageEstimator.sizeOf(tagValue) + INDEX_ENTRY_OVERHEAD_ESTIMATE_BYTES;
186+
}
182187

183-
long memorySize = 0;
184-
if (tagIndexNewSize - tagIndexOldSize == 1) {
185-
// the last 4 is the memory occupied by the size of tagvaluemap
186-
memorySize += RamUsageEstimator.sizeOf(tagKey) + 4;
187-
}
188-
if (tagValueMapNewSize - tagValueMapOldSize == 1) {
189-
// the last 4 is the memory occupied by the size of measurementsSet
190-
memorySize += RamUsageEstimator.sizeOf(tagValue) + 4;
191-
}
192-
if (measurementsSetNewSize - measurementsSetOldSize == 1) {
193-
// 8 is the memory occupied by the length of the IMeasurementMNode
194-
memorySize += RamUsageEstimator.NUM_BYTES_OBJECT_REF + 4;
195-
}
196-
requestMemory(memorySize);
188+
if (measurementsSet.add(measurementMNode)) {
189+
memorySize +=
190+
RamUsageEstimator.NUM_BYTES_OBJECT_REF + INDEX_ENTRY_OVERHEAD_ESTIMATE_BYTES;
191+
}
192+
if (memorySize > 0) {
193+
requestMemory(memorySize);
194+
}
195+
return tagValueMap;
196+
});
197197
}
198198

199199
public void addIndex(Map<String, String> tagsMap, IMeasurementMNode<?> measurementMNode) {
@@ -208,32 +208,47 @@ public void removeIndex(String tagKey, String tagValue, IMeasurementMNode<?> mea
208208
if (tagKey == null || tagValue == null || measurementMNode == null) {
209209
return;
210210
}
211-
// init memory size
212-
long memorySize = 0;
213-
if (tagIndex.get(tagKey).get(tagValue).remove(measurementMNode)) {
214-
memorySize += RamUsageEstimator.NUM_BYTES_OBJECT_REF + 4;
215-
}
216-
if (tagIndex.get(tagKey).get(tagValue).isEmpty()) {
217-
if (tagIndex.get(tagKey).remove(tagValue) != null) {
218-
// the last 4 is the memory occupied by the size of IMeasurementMNodeSet
219-
memorySize += RamUsageEstimator.sizeOf(tagValue) + 4;
220-
}
221-
}
222-
if (tagIndex.get(tagKey).isEmpty()) {
223-
if (tagIndex.remove(tagKey) != null) {
224-
// the last 4 is the memory occupied by the size of tagValueMap
225-
memorySize += RamUsageEstimator.sizeOf(tagKey) + 4;
226-
}
227-
}
228-
releaseMemory(memorySize);
211+
tagIndex.computeIfPresent(
212+
tagKey,
213+
(key, tagValueMap) -> {
214+
long memorySize = 0;
215+
Set<IMeasurementMNode<?>> measurementsSet = tagValueMap.get(tagValue);
216+
if (measurementsSet == null) {
217+
return tagValueMap;
218+
}
219+
220+
if (measurementsSet.remove(measurementMNode)) {
221+
memorySize +=
222+
RamUsageEstimator.NUM_BYTES_OBJECT_REF + INDEX_ENTRY_OVERHEAD_ESTIMATE_BYTES;
223+
}
224+
if (measurementsSet.isEmpty()) {
225+
if (tagValueMap.remove(tagValue, measurementsSet)) {
226+
memorySize +=
227+
RamUsageEstimator.sizeOf(tagValue) + INDEX_ENTRY_OVERHEAD_ESTIMATE_BYTES;
228+
}
229+
}
230+
if (tagValueMap.isEmpty()) {
231+
memorySize += RamUsageEstimator.sizeOf(tagKey) + INDEX_ENTRY_OVERHEAD_ESTIMATE_BYTES;
232+
if (memorySize > 0) {
233+
releaseMemory(memorySize);
234+
}
235+
return null;
236+
}
237+
if (memorySize > 0) {
238+
releaseMemory(memorySize);
239+
}
240+
return tagValueMap;
241+
});
242+
}
243+
244+
private boolean containsIndex(String tagKey, String tagValue) {
245+
Map<String, Set<IMeasurementMNode<?>>> tagValueMap = tagIndex.get(tagKey);
246+
return tagValueMap != null && tagValueMap.containsKey(tagValue);
229247
}
230248

231249
private List<IMeasurementMNode<?>> getMatchedTimeseriesInIndex(TagFilter tagFilter) {
232-
if (!tagIndex.containsKey(tagFilter.getKey())) {
233-
return Collections.emptyList();
234-
}
235250
Map<String, Set<IMeasurementMNode<?>>> value2Node = tagIndex.get(tagFilter.getKey());
236-
if (value2Node.isEmpty()) {
251+
if (value2Node == null || value2Node.isEmpty()) {
237252
return Collections.emptyList();
238253
}
239254

@@ -364,8 +379,7 @@ public void removeFromTagInvertedIndex(IMeasurementMNode<?> node) throws IOExcep
364379
Map<String, String> tagMap = tagLogFile.readTag(node.getOffset());
365380
if (tagMap != null) {
366381
for (Map.Entry<String, String> entry : tagMap.entrySet()) {
367-
if (tagIndex.containsKey(entry.getKey())
368-
&& tagIndex.get(entry.getKey()).containsKey(entry.getValue())) {
382+
if (containsIndex(entry.getKey(), entry.getValue())) {
369383
if (logger.isDebugEnabled()) {
370384
logger.debug(
371385
String.format(
@@ -418,7 +432,7 @@ public void updateTagsAndAttributes(
418432
// we should remove before key-value from inverted index map
419433
if (beforeValue != null && !beforeValue.equals(value)) {
420434

421-
if (tagIndex.containsKey(key) && tagIndex.get(key).containsKey(beforeValue)) {
435+
if (containsIndex(key, beforeValue)) {
422436
if (logger.isDebugEnabled()) {
423437
logger.debug(
424438
String.format(
@@ -551,8 +565,7 @@ public void dropTagsOrAttributes(
551565

552566
if (!deleteTag.isEmpty()) {
553567
for (Map.Entry<String, String> entry : deleteTag.entrySet()) {
554-
if (tagIndex.containsKey((entry.getKey()))
555-
&& tagIndex.get(entry.getKey()).containsKey(entry.getValue())) {
568+
if (containsIndex(entry.getKey(), entry.getValue())) {
556569
if (logger.isDebugEnabled()) {
557570
logger.debug(
558571
String.format(
@@ -622,7 +635,7 @@ public void setTagsOrAttributesValue(
622635
String beforeValue = entry.getValue();
623636
String currentValue = newTagValue.get(key);
624637
// change the tag inverted index map
625-
if (tagIndex.containsKey(key) && tagIndex.get(key).containsKey(beforeValue)) {
638+
if (containsIndex(key, beforeValue)) {
626639

627640
if (logger.isDebugEnabled()) {
628641
logger.debug(
@@ -680,7 +693,7 @@ public void renameTagOrAttributeKey(
680693
// persist the change to disk
681694
tagLogFile.write(pair.left, pair.right, leafMNode.getOffset());
682695
// change the tag inverted index map
683-
if (tagIndex.containsKey(oldKey) && tagIndex.get(oldKey).containsKey(value)) {
696+
if (containsIndex(oldKey, value)) {
684697

685698
if (logger.isDebugEnabled()) {
686699
logger.debug(

0 commit comments

Comments
 (0)