Let the DataPartitionTable be automatically cleanable by CRZbulabula · Pull Request #14737 · apache/iotdb

CRZbulabula · 2025-01-20T14:29:52Z

A data partition is not necessary to exist when all corresponding data are expired given the pre-configurated TTL. Hence, this PR added a thread to automatically clean these expired data partitions, making the cache size of the DataPartitionTable is always acceptable! Specifically, the main updates include:

In TTLManager, to retrieve the maximum TTL of a specific database, we implement getDatabaseMaxTTL().
In TimePartitionUtils, to locate the time partition slot of the current timestamp, we implement the getCurrentTimePartitionSlot().
In PartitionManager, we setup a periodically asynchronous thread, which combines the aforementioned interfaces to construct an "AutoCleanPartitionTablePlan." As a result, for each database, any data partition whose data are all expired will be deleted automatically by this independent thread.

Incidentally, the corresponding IT is available at "IoTDBPartitionTableAutoCleanTest."

codecov · 2025-01-20T15:40:14Z

Codecov Report

Attention: Patch coverage is 41.02564% with 69 lines in your changes missing coverage. Please review.

Project coverage is 39.21%. Comparing base (bc5fdae) to head (8711a10).
Report is 25 commits behind head on master.

Files with missing lines	Patch %	Lines
...onfignode/procedure/PartitionTableAutoCleaner.java	20.00%	20 Missing ⚠️
.../org/apache/iotdb/commons/schema/ttl/TTLCache.java	0.00%	14 Missing ⚠️
...onfignode/persistence/partition/PartitionInfo.java	0.00%	8 Missing ⚠️
...t/write/partition/AutoCleanPartitionTablePlan.java	84.37%	5 Missing ⚠️
...apache/iotdb/commons/utils/TimePartitionUtils.java	16.66%	5 Missing ⚠️
.../iotdb/commons/partition/SeriesPartitionTable.java	20.00%	4 Missing ⚠️
...g/apache/iotdb/confignode/persistence/TTLInfo.java	0.00%	3 Missing ⚠️
...he/iotdb/commons/partition/DataPartitionTable.java	0.00%	3 Missing ⚠️
...che/iotdb/confignode/manager/ProcedureManager.java	33.33%	2 Missing ⚠️
.../persistence/partition/DatabasePartitionTable.java	0.00%	2 Missing ⚠️
... and 3 more

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #14737      +/-   ##
============================================
+ Coverage     39.12%   39.21%   +0.09%     
  Complexity      193      193              
============================================
  Files          4417     4445      +28     
  Lines        281267   282533    +1266     
  Branches      34783    34861      +78     
============================================
+ Hits         110044   110800     +756     
- Misses       171223   171733     +510

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

liyuheng55555

Haven't finished reading all the code yet, I have some thoughts for discussing:

If the default execution interval is every 2 hours, then maybe adding a periodically executed Procedure is more lightweight compared to adding a new thread pool?

liyuheng55555 · 2025-01-21T09:41:13Z

...src/test/java/org/apache/iotdb/confignode/it/partition/IoTDBPartitionTableAutoCleanTest.java

+      for (int retry = 0; retry < 120; retry++) {
+        boolean partitionTableAutoCleaned = true;
+        TDataPartitionTableResp resp = client.getDataPartitionTable(req);
+        if (TSStatusCode.SUCCESS_STATUS.getStatusCode() == resp.getStatus().getCode()) {
+          Map<String, Map<TSeriesPartitionSlot, Map<TTimePartitionSlot, List<TConsensusGroupId>>>>
+              dataPartitionTable = resp.getDataPartitionTable();
+          for (Map.Entry<
+                  String,
+                  Map<TSeriesPartitionSlot, Map<TTimePartitionSlot, List<TConsensusGroupId>>>>
+              e1 : dataPartitionTable.entrySet()) {
+            for (Map.Entry<TSeriesPartitionSlot, Map<TTimePartitionSlot, List<TConsensusGroupId>>>
+                e2 : e1.getValue().entrySet()) {
+              if (e2.getValue().size() != 1) {
+                // The PartitionTable of each database should only contain 1 time partition slot
+                partitionTableAutoCleaned = false;
+                break;
+              }
+            }
+            if (!partitionTableAutoCleaned) {
+              break;
+            }
+          }
+        }
+        if (partitionTableAutoCleaned) {
+          return;
+        }
+        TimeUnit.SECONDS.sleep(1);


Considering using Awaitility to replace the outer for-loop and sleep

(As Chatgpt suggested :) please checkout whether these two inner loop can be simplified, such as:

partitionTableAutoCleaned = resp.getDataPartitionTable().entrySet().stream() .flatMap(e1 -> e1.getValue().entrySet().stream()) .allMatch(e2 -> e2.getValue().size() == 1);

An awesome suggestion! The corresponding test codes are simplified significantly!

iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/concurrent/ThreadName.java

OneSizeFitsQuorum · 2025-01-22T04:21:18Z

...confignode/src/main/java/org/apache/iotdb/confignode/manager/partition/PartitionManager.java

+    for (String database : databases) {
+      long subTreeMaxTTL = getTTLManager().getDatabaseMaxTTL(database);
+      databaseTTLMap.put(
+          database, Math.max(subTreeMaxTTL, databaseTTLMap.getOrDefault(database, -1L)));


add some judgement like "isDatabaseExisted(database) && 0 < ttl && ttl < Long.MAX_VALUE" here to remove overhead?
BTW, If all the databases don't have ttl, we can logically just do this check and find that none of them need to be cleaned up, so there's no need to do a consensus write

Thanks for pinpointing this logic enhancement. The judgement is available at PartitionTableAutoCleaner.

OneSizeFitsQuorum · 2025-01-22T04:21:54Z

...confignode/src/main/java/org/apache/iotdb/confignode/manager/partition/PartitionManager.java

    this.regionMaintainer =
        IoTDBThreadPoolFactory.newSingleThreadScheduledExecutor(
            ThreadName.CONFIG_NODE_REGION_MAINTAINER.getName());
+    this.partitionCleaner =


maybe try to reuse procedure periodic tasks

Sure. I now employ the PartitionTableAutoCleaner rather than creating an extra thread pool.

...src/test/java/org/apache/iotdb/confignode/it/partition/IoTDBPartitionTableAutoCleanTest.java

sonarqubecloud · 2025-01-22T07:39:55Z

Quality Gate passed

Issues
31 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

liyuheng55555

Have you considered concurrency issues, such as setTTL and Cleaner running simultaneously?

(I think one solution could be to clean up partitions only after they’ve exceeded the TTL by one hour, but you might have solved this in another way ：)

CRZbulabula · 2025-01-22T08:52:57Z

Have you considered concurrency issues, such as setTTL and Cleaner running simultaneously?

(I think one solution could be to clean up partitions only after they’ve exceeded the TTL by one hour, but you might have solved this in another way ：)

Thanks for raising this critical issue. To address your concern, please refer to the autoCleanPartitionTable in the SeriesPartitionTable, as outlined below:

public void autoCleanPartitionTable(long TTL, TTimePartitionSlot currentTimeSlot) {
    seriesPartitionMap
        .entrySet()
        .removeIf(entry -> entry.getKey().getStartTime() + TTL < currentTimeSlot.getStartTime());
  }

Here, the removing condition is < rather than <=. As a result, a data partition is removed from cache only when it exceeded the TTL by a whole time partitioning interval. Therefore, the PartitionTable always contains a row of "empty" data partitions. I believe this approach can avoid this concurrency problem.

liyuheng55555

Good work! Thank you for addressing my feedback patiently!

OneSizeFitsQuorum

LGTM

* seems finished * Use periodic procedure 4 partition table cleaner * Update ThreadName.java

…14737) (#14759) * Let the DataPartitionTable be automatically cleanable (#14737) * seems finished * Use periodic procedure 4 partition table cleaner * Update ThreadName.java * Update PartitionTableAutoCleaner.java

…pache#14737) (apache#14759) * Let the DataPartitionTable be automatically cleanable (apache#14737) * seems finished * Use periodic procedure 4 partition table cleaner * Update ThreadName.java * Update PartitionTableAutoCleaner.java

seems finished

1839533

liyuheng55555 requested changes Jan 21, 2025

View reviewed changes

OneSizeFitsQuorum reviewed Jan 22, 2025

View reviewed changes

CRZbulabula added 2 commits January 22, 2025 15:29

Use periodic procedure 4 partition table cleaner

cfd9432

Update ThreadName.java

8711a10

liyuheng55555 reviewed Jan 22, 2025

View reviewed changes

liyuheng55555 approved these changes Jan 23, 2025

View reviewed changes

OneSizeFitsQuorum approved these changes Jan 23, 2025

View reviewed changes

OneSizeFitsQuorum merged commit 3369e7a into master Jan 23, 2025

OneSizeFitsQuorum deleted the auto-clean-partition-table branch January 23, 2025 02:27

CRZbulabula added a commit that referenced this pull request Jan 23, 2025

Let the DataPartitionTable be automatically cleanable (#14737)

bfe4bd6

* seems finished * Use periodic procedure 4 partition table cleaner * Update ThreadName.java

Conversation

CRZbulabula commented Jan 20, 2025

Uh oh!

codecov bot commented Jan 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

liyuheng55555 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liyuheng55555 Jan 21, 2025

Choose a reason for hiding this comment

Uh oh!

CRZbulabula Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

OneSizeFitsQuorum Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

CRZbulabula Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

OneSizeFitsQuorum Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

CRZbulabula Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Jan 22, 2025

Quality Gate passed

Uh oh!

liyuheng55555 left a comment

Choose a reason for hiding this comment

Uh oh!

CRZbulabula commented Jan 22, 2025

Uh oh!

liyuheng55555 left a comment

Choose a reason for hiding this comment

Uh oh!

OneSizeFitsQuorum left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Jan 20, 2025 •

edited

Loading

liyuheng55555 left a comment •

edited

Loading