Enable auto merging of tablets #5353

cshannon · 2025-02-23T20:27:08Z

This change adds support for periodically scanning tables to find ranges of tables that can be automatically merged. The thread that runs uses the recently added TabletMergeabilty column as well as total file counts/sizes to compute if a range can be merged. By default, tablets can be merged if the total size is less than 25% of the split threshold so we don't immediately merge only to split again.

There is a new thread in the Manager class that performs the computation and will submit fate jobs for merging on tablet ranges. There is a new fate operation during merge that will validate the range is still ok to merge if the merge was submitted as a system merge.

This closes #5014

I am making this a draft because while it's feature complete, I still think there's some more work to do on the tests and tweaks to make, but wanted to get some feedback/suggestions.

Outstanding issues/note/questions:

For user created tables, the default tablet was previously set to never merge, but I think we should change this to always so i switched the default tablet to be a mergeability of always in this PR. The reason is we can split the default tablet, so we should be able to merge other tablets back to it. It will never go away because the end row is null. One reason why I did this is there is no way in the API right now to change the mergeability for the default tablet (end row is null which you can't put into a map as a key) so I figure we should just make it always mergeable and any other splits would not be mergeable by default for user created tablets. This would be easy to set back to never by default if desired, but if keep the default tablet as never then we can never merge back to a single tablet again.
The default tables (metadata, fate, scanref) are configured with their initital tablet(s) to have a mergeability setting of never. Should we change this? I am thinking I probably want to switch the default tablet to be mergeable just like I did for user tables for consistency. However, pre-splitting is ever done the tablets would need to be made sure to be marked as never merge so they don't get merged away due to a size of 0.
What thread pool should we use for FindMergeableRangeTask? What interval by default? For now I put in a todo marker and just used the default context scheduler and set a run time of every 24 hours.
As mentioned, the tests could definitely be improved with more edge cases and complexity. The TabletMergeabilityIT especially only has 4 tests for now to demonstrate but there's a lot more tests we could write to verify the checks work like max files, sizes, etc work and we get the exepcted merges done.

This change adds support for periodically scanning tables to find ranges of tables that can be automatically merged. The thread that runs uses the recently added TabletMergeabilty column as well as total file counts/sizes to compute if a range can be merged. By default, tablets can be merged if the total size is less than 25% of the split threshold so we don't immediately merge only to split again. There is a new thread in the Manager class that performs the computation and will submit fate jobs for merging on tablet ranges. There is a new fate operation during merge that will validate the range is still ok to merge if the merge was submitted as a system merge. This closes apache#5014

cshannon · 2025-02-23T20:38:29Z

@keith-turner and @dlmarion - I gave details in the description but even though this is marked as a draft it's still ready for review and feedback.

cshannon · 2025-02-24T13:20:39Z

Looks like there's a couple tests to fix which I can do when I get a chance, they are boken cause the PR changes the default tablet mergeability setting to always instead of never as mentioned in the description.

org.apache.accumulo.test.functional.CreateInitialSplitsIT.testCreateInitialSplitsWithMergeability
org.apache.accumulo.test.functional.SplitIT.tabletShouldSplit

Edit: Fixed tests in fc0dbec

core/src/main/java/org/apache/accumulo/core/conf/Property.java

core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java

dlmarion · 2025-02-24T14:00:23Z

server/manager/src/main/java/org/apache/accumulo/manager/Manager.java

@@ -1292,6 +1293,13 @@ boolean canSuspendTablets() {
    ThreadPools.watchCriticalScheduledTask(context.getScheduledExecutor()
        .scheduleWithFixedDelay(() -> ScanServerMetadataEntries.clean(context), 10, 10, MINUTES));

+    // TODO - create new threadpool?


I assume that this cannot go into the TabletGroupWatcher because it needs to look at more than one Tablet? I think using the general scheduled executor threadpool should be fine. However, we may want to consider bumping the default value of GENERAL_THREADPOOL_SIZE from 1 to something larger. Probably good for a separate PR.

Yes, TGW doesn't work in this case because because the task needs to look at multiple tablets together and not just one tablet at a time. Also, the task likely doesn't need to run very often to look for mergeable ranges where as TGW runs frequently

server/manager/src/main/java/org/apache/accumulo/manager/merge/FindMergeableRangeTask.java

dlmarion · 2025-02-24T14:28:42Z

Is there work to do in the upgrade code, or has that already been done?

cshannon · 2025-02-25T00:07:09Z

Is there work to do in the upgrade code, or has that already been done?

There is still upgrade work to do, we should go through and mark all existing tablets as "never" merge on upgrade. If I recall the default will be never if the column is absent but I think it's better to be explicit and mark the tablets.

server/manager/src/main/java/org/apache/accumulo/manager/merge/FindMergeableRangeTask.java

keith-turner · 2025-02-25T21:03:31Z

server/manager/src/main/java/org/apache/accumulo/manager/tableOps/merge/VerifyMergeability.java

+      long totalFiles = 0;
+      int totalUnMergeable = 0;
+
+      for (var tabletMetadata : tablets) {


When I first saw this code I though maybe it could use the MergeableRange code and try to add all tablets in the range to that. However that would not give the reason that a tablet could not be added, so it almost works but not quite.

I thought the same thing, I have/had been trying to figure out a good way to share the same logic between both but it's just different enough there wasn't really a good way to share the code but i'll keep thinking about it.

Wondering if MergeableRange.add() returned an enum instead of a boolean if that could work. The enum would indicate if added or if not added then why.

I refactored things to share MergeableRange between the task and verify in 3c84e85

core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java

server/manager/src/main/java/org/apache/accumulo/manager/merge/FindMergeableRangeTask.java

test/src/main/java/org/apache/accumulo/test/functional/TabletMergeabilityIT.java

keith-turner · 2025-03-01T16:27:19Z

server/manager/src/main/java/org/apache/accumulo/manager/merge/FindMergeableRangeTask.java


-    manager.fate(type).seedTransaction(FateOperation.SYSTEM_MERGE, fateId,
+    manager.fate(type).seedTransaction(FateOperation.SYSTEM_MERGE, fateKey,


This will wait for the data to be written to the walog of the fate table for each operation seeded. W/ the changes in #5160 this is another place where waiting on the walog per operation can be avoided.

keith-turner · 2025-03-01T16:54:35Z

test/src/main/java/org/apache/accumulo/test/fate/FateStoreIT.java

@@ -683,8 +689,12 @@ protected void testListFateKeys(FateStore<TestEnv> store, ServerContext sctx) th
    var fateKey3 = FateKey.forCompactionCommit(cid1);
    var fateKey4 = FateKey.forCompactionCommit(cid2);

+    // use one overlapping extent and one different
+    var fateKey5 = FateKey.forMerge(extent1);


Not a change for this PR. Was looking around to see if FateKey had any unit test that should be updated and did not see any. Could be useful to add some unit test to check equals, hashcode, serder, etc in another PR.

test/src/main/java/org/apache/accumulo/test/functional/TabletMergeabilityIT.java

cshannon · 2025-03-09T18:43:17Z

I ran the full IT build and it passed so this is ready for another review.

cshannon requested review from dlmarion and keith-turner February 23, 2025 20:27

cshannon force-pushed the accumulo-5014-merge-thread branch from 0e8e73b to c089d87 Compare February 23, 2025 20:32

cshannon self-assigned this Feb 23, 2025

cshannon added this to the 4.0.0 milestone Feb 23, 2025

dlmarion reviewed Feb 24, 2025

View reviewed changes

cshannon added 2 commits February 24, 2025 18:42

Merge branch 'main' into accumulo-5014-merge-thread

302848a

Address some comments and fix tests

fc0dbec

Address more comments

81f2c37

keith-turner reviewed Feb 25, 2025

View reviewed changes

cshannon added 3 commits February 28, 2025 07:42

Merge branch 'main' into accumulo-5014-merge-thread

3d51411

Address comments

a8d193a

Add new MERGE FateKey type

62254bc

keith-turner reviewed Mar 1, 2025

View reviewed changes

cshannon added 5 commits March 1, 2025 12:17

Refactor to reuse Mergeablerange inside VerifyMergeability

3c84e85

skip table check if mergeability threshold is <= 0

eb991fc

Merge branch 'main' into accumulo-5014-merge-thread

e588050

Add IT for table max mergeability threshold

226dd58

Merge branch 'main' into accumulo-5014-merge-thread

71a31e4

keith-turner reviewed Mar 8, 2025

View reviewed changes

test/src/main/java/org/apache/accumulo/test/functional/TabletMergeabilityIT.java Show resolved Hide resolved

cshannon added 2 commits March 8, 2025 13:29

more IT updates

ec466fa

fix junit import

e45a2ed

cshannon marked this pull request as ready for review March 8, 2025 18:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable auto merging of tablets #5353

Enable auto merging of tablets #5353

cshannon commented Feb 23, 2025

cshannon commented Feb 23, 2025

cshannon commented Feb 24, 2025 •

edited

Loading

dlmarion Feb 24, 2025

cshannon Feb 24, 2025

dlmarion commented Feb 24, 2025

cshannon commented Feb 25, 2025

keith-turner Feb 25, 2025

cshannon Feb 25, 2025

keith-turner Feb 25, 2025

cshannon Mar 1, 2025

keith-turner Mar 1, 2025

keith-turner Mar 1, 2025

cshannon commented Mar 9, 2025


		manager.fate(type).seedTransaction(FateOperation.SYSTEM_MERGE, fateId,
		manager.fate(type).seedTransaction(FateOperation.SYSTEM_MERGE, fateKey,

Enable auto merging of tablets #5353

Are you sure you want to change the base?

Enable auto merging of tablets #5353

Conversation

cshannon commented Feb 23, 2025

Outstanding issues/note/questions:

cshannon commented Feb 23, 2025

cshannon commented Feb 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dlmarion commented Feb 24, 2025

cshannon commented Feb 25, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cshannon commented Mar 9, 2025

cshannon commented Feb 24, 2025 •

edited

Loading