Submit delete batches in parallel by poodlewars · Pull Request #3123 · man-group/ArcticDB

poodlewars · 2026-05-19T14:16:21Z

Discovered while working on delayed deletes. I created a library on PURE with 1000 symbols, with ~200 versions each and 10 snapshots.

Then running delayed deletes had these results:

Side	Elapsed (s)	Peak RSS (MiB)
arcticdb HEAD (47bba8011, parallel batches)	349.9648	780.3
arcticdb `2d1e60e` (pre-batching)	399.6425	792.9
arcticc	333.3081	421.3

Notably running delayed deletes in "dry run" mode (which skips the physical deletion) showed no performance difference between arcticc and ArcticDB, for example on a small example:

Side	Mode	Elapsed (s)	Peak RSS (MiB)
arcticdb	dry-run	6.4449	388.2
arcticdb	real	19.2177	427.9
arcticc	dry-run	8.6100	329.0
arcticc	real	11.2915	330.4

arcticc submitted RemoveBatchTask in chunks of 1000 https://mangit.maninvestments.com/projects/DATA/repos/arcticc/browse/arcticcxx_impl/async/async_store.hpp#238 but #70 changed it to this implementation.

If we delete 10k keys, the old implementation in arcticc would submit 10 tasks to delete 1000 keys in parallel, whereas ArcticDB currently submits 10 HTTP requests to delete 1000 keys in serial.

This explains most of the performance difference between arcticc delayed deletes and enterprise delayed deletes.

To stop storage specific deletion batch size limits leaking out of the storage layer, this PR has each storage declare its maximum deletion batch size, which is then used by the AsyncStore when it calculates the batches.

claude · 2026-05-19T15:20:25Z

ArcticDB Code Review Summary

Build and Dependencies

FIXED in commit 31f99ea: cpp/arcticdb/entity/test/test_variant_key.cpp has now been added with unit tests covering key_types() (empty, single type, multiple deduped types, atom+ref mix). CMakeLists.txt entry now resolves.

API and Compatibility

The pre-existing Storage.DeleteBatchSize config key (was read in cpp/arcticdb/util/key_utils.hpp) is replaced in this PR by Storage.DeletePendingBufferSize (different semantics: accumulator/flush threshold rather than per-request batch size). The actual storage batch size is now controlled by the existing S3Storage.DeleteBatchSize and AzureStorage.DeleteBatchSize configs via the new max_delete_batch_size() path. Any externally set Storage.DeleteBatchSize will be silently ignored; either preserve backward compatibility, surface a warning, or call out this rename in the description or release notes.

Documentation

Add a release note for this behaviour change. The patch and performance labels are set but no-release-notes is not, so users should be informed that delete throughput improves and that the Storage.DeleteBatchSize config key is no longer honoured.

alexowens90 · 2026-05-29T09:25:21Z


+template<std::ranges::range R>
+requires std::same_as<std::remove_cvref_t<std::ranges::range_value_t<R>>, VariantKey>
+std::vector<KeyType> key_types(R&& keys) {


Feels like the return type of this should be an unordered set?

alexowens90 · 2026-05-29T09:39:42Z

+    for (auto& k : variant_keys) {
+        auto collection = collection_name(variant_key_type(k));
+        try {
+            auto result = client_->remove_keyvalue(db_, collection, k);


Does the Mongo client not have a method to delete multiple documents (at least in the same collection) at once?

alexowens90 · 2026-05-29T09:40:32Z

  public:
    GCPXMLStorage(const LibraryPath& lib, OpenMode mode, const GCPXMLSettings& conf);

+    std::optional<size_t> max_delete_batch_size() const override { return std::nullopt; }


Google has no limit!?

alexowens90 · 2026-05-29T09:42:36Z

+    for (const auto& k : ks) {
+        auto key_type_dir = key_type_folder(root_folder, variant_key_type(k));
+        to_delete.emplace_back(object_path(bucketizer.bucketize(key_type_dir, k), k));
+    }


Could use std::ranges::transform

alexowens90 · 2026-05-29T09:45:02Z

+        auto distinct_key_types = key_types(ks);
+        stat_timers.reserve(distinct_key_types.size());
+        for (auto kt : distinct_key_types) {
+            stat_timers.emplace_back(query_stats::add_task_count_and_time(query_stats::TaskType::S3_DeleteObjects, kt));


I guess the query stats will now look odd for a batch-delete of mixed key types, but I'm not sure users can even trigger this in practice so not worth worrying about

alexowens90 · 2026-05-29T09:47:54Z

+
+    static std::vector<std::vector<entity::VariantKey>> chunk_keys(
+            std::vector<entity::VariantKey>&& keys, size_t batch_size
+    ) {


This is a pattern we have dotted all over the place, could template the element type and stick it in the utils folder somewhere

alexowens90 · 2026-05-29T09:53:30Z

+        );
+        return folly::collect(std::move(futs)).via(&async::io_executor()).thenValue([](auto&&) {
+            return std::vector<RemoveKeyResultType>{};
+        });


https://github.com/man-group/ArcticDB/blob/master/cpp/arcticdb/stream/stream_sink.hpp#L49-L54
Don't have to, but if you want to rip out this pointless abstraction while you're here I won't object

alexowens90 · 2026-05-29T10:06:23Z

+                        keys = {};
+                        keys.reserve(flush_threshold);
+                        const auto batch_size = batch.size();
+                        store->remove_keys(std::move(batch)).get();


There is a slightly more elegant model that would remove the need for flush_threshold . Instead of calling get immediately, we maintain a set of remove_keys futures we've submitted. Folly let's you check if a future is done without calling get() using isReady(), so the size of this collection could be kept bounded. It's quite a bit more complicated though, and probably doesn't add much in this case.
At least worth a comment about why remove_keys is better than remove_keys_sync in this case though.

alexowens90 · 2026-05-29T10:14:51Z

+    with (
+        config_context("S3Storage.DeleteBatchSize", batch_size),
+        config_context("AzureStorage.DeleteBatchSize", batch_size),
+    ):


There's a config_context_multi for setting multiple options at once

alexowens90 · 2026-05-29T10:17:08Z

+
+
+@pytest.mark.storage
+def test_bulk_delete_batching(object_and_mem_and_lmdb_version_store, sym):


This is quite an indirect test of the batching. I get that we don't have query stats for Azure right now, but the test below is much more explicit

poodlewars added patch Small change, should increase patch version performance labels May 19, 2026

poodlewars force-pushed the deletion-cleanup branch from 5094aad to 25c3c5a Compare May 19, 2026 15:12

poodlewars marked this pull request as ready for review May 19, 2026 15:14

poodlewars requested review from IvoDD and alexowens90 as code owners May 19, 2026 15:14

poodlewars force-pushed the deletion-cleanup branch from 25c3c5a to 9852aeb Compare May 19, 2026 16:33

Submit delete batches in parallel

0614e82

poodlewars force-pushed the deletion-cleanup branch from 9852aeb to 0614e82 Compare May 19, 2026 16:34

G-D-Petrov reviewed May 21, 2026

View reviewed changes

Comment thread cpp/arcticdb/storage/s3/nfs_backed_storage.cpp Outdated

G-D-Petrov approved these changes May 21, 2026

View reviewed changes

claude Bot reviewed May 22, 2026

View reviewed changes

Comment thread cpp/arcticdb/CMakeLists.txt

Alex Seaton added 3 commits May 22, 2026 14:07

Remove folly::gen from storage backends

54ebb3e

Use proper default value

3183f28

Use proper default value

31f99ea

poodlewars force-pushed the deletion-cleanup branch from c7ded90 to 31f99ea Compare May 22, 2026 13:09

alexowens90 reviewed May 29, 2026

View reviewed changes

alexowens90 approved these changes May 29, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Submit delete batches in parallel#3123

Submit delete batches in parallel#3123
poodlewars wants to merge 4 commits into
masterfrom
deletion-cleanup

poodlewars commented May 19, 2026 •

edited

Loading

Uh oh!

claude Bot commented May 19, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

alexowens90 May 29, 2026

Uh oh!

alexowens90 May 29, 2026

Uh oh!

alexowens90 May 29, 2026

Uh oh!

alexowens90 May 29, 2026

Uh oh!

alexowens90 May 29, 2026

Uh oh!

alexowens90 May 29, 2026

Uh oh!

alexowens90 May 29, 2026

Uh oh!

alexowens90 May 29, 2026 •

edited

Loading

Uh oh!

alexowens90 May 29, 2026

Uh oh!

alexowens90 May 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants



		@pytest.mark.storage
		def test_bulk_delete_batching(object_and_mem_and_lmdb_version_store, sym):

Conversation

poodlewars commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ArcticDB Code Review Summary

Build and Dependencies

API and Compatibility

Documentation

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexowens90 May 29, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

poodlewars commented May 19, 2026 •

edited

Loading

claude Bot commented May 19, 2026 •

edited

Loading

alexowens90 May 29, 2026 •

edited

Loading