You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "JingsongLi (via GitHub)" <gi...@apache.org> on 2026/07/27 04:57:32 UTC
[PR] [core] Improve Simple LSM lookup performance [paimon]
JingsongLi opened a new pull request, #8857:
URL: https://github.com/apache/paimon/pull/8857
## What changed
- enable dynamically sized per-SST Bloom filters for Simple LSM flush, bulk-load, and compaction output
- reduce the default local KV block size from 32 KB to 4 KB and expose `local-kv-db.block-size`
- use Caffeine for the primary-key clustering local KV cache while preserving Guava elsewhere
- reduce hot-path allocations by caching cache-key hash codes and avoiding entry materialization during block seeks
- add focused correctness tests and a configurable Simple LSM versus RocksDB benchmark
## Why
Random point lookups in a local KV database larger than its cache were dominated by block read amplification and cache lookup overhead. Missing-key lookups also visited SST files unnecessarily when no Bloom filter was available.
## Impact
With 3 million records, 1 million operations, a 128 MB cache, 4 KB blocks, LZ4, and Bloom-filter FPP 0.1:
| Workload | Simple LSM | RocksDB |
| --- | ---: | ---: |
| point lookup hit (best) | 873 K ops/s | 523 K ops/s |
| point lookup miss (best) | 6002 K ops/s | 2773 K ops/s |
| 50/50 mixed read/write (best) | 482 K ops/s | 356 K ops/s |
| database size | 214.55 MB | 445.34 MB |
The mixed workload still shows periodic compaction latency, so its average throughput remains roughly level with RocksDB.
## Validation
- `SimpleLsmKvDbTest`: 51 tests passed
- `CoreOptionsTest`, `MemoryFileStoreWriteTest`, and `ClusteringTableTest`: 60 tests passed
- strict compile and test-compile passed for the 15-module micro-benchmark reactor
- Simple LSM versus RocksDB point-lookup and mixed read/write benchmarks passed
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Improve local KV database lookup performance [paimon]
Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
leaves12138 commented on code in PR #8857:
URL: https://github.com/apache/paimon/pull/8857#discussion_r3655591048
##########
paimon-common/src/main/java/org/apache/paimon/lookup/sort/db/AsyncLsmCompactor.java:
##########
@@ -0,0 +1,200 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.paimon.lookup.sort.db;
+
+import javax.annotation.Nullable;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.CancellationException;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+
+/** Coordinates asynchronous compaction, failure propagation, backpressure, and shutdown. */
+class AsyncLsmCompactor {
+
+ private final LsmLevels levels;
+ private final LsmCompactor compactor;
+ private final ExecutorService executor;
+ private final int levelZeroFileCountTrigger;
+ private final LsmCompactor.FileSupplier fileSupplier;
+ private final LsmCompactor.FileDeleter fileDeleter;
+ private final ThreadLocal<List<File>> deferredCompactionDeletes;
+
+ @Nullable private Future<?> compactionFuture;
+
+ AsyncLsmCompactor(
+ LsmLevels levels,
+ CompactorFactory compactorFactory,
+ ExecutorService executor,
+ int levelZeroFileCountTrigger,
+ LsmCompactor.FileSupplier fileSupplier,
+ LsmCompactor.FileDeleter fileDeleter) {
+ this.levels = levels;
+ this.executor = executor;
+ this.levelZeroFileCountTrigger = levelZeroFileCountTrigger;
+ this.fileSupplier = fileSupplier;
+ this.fileDeleter = fileDeleter;
+ this.deferredCompactionDeletes = new ThreadLocal<>();
+ this.compactor = compactorFactory.create(this::deferOrDeleteCompactedFile);
+ }
+
+ void scheduleIfNeeded() throws IOException {
+ checkFailure();
+ if (!needsCompaction() || compactionFuture != null) {
+ return;
+ }
+
+ try {
+ compactionFuture =
+ executor.submit(
+ () -> {
+ while (needsCompaction()) {
Review Comment:
Could we reject non-positive `level0FileNumCompactTrigger` values before starting asynchronous compaction? The public builder currently accepts `0` or a negative value. With `0`, `flush()` schedules the worker and then always enters backpressure (`L0 file count >= 0`), while the worker loops forever because `needsCompaction()` remains true even after L0 becomes empty. A minimal repro prints `beforeFlush` and still does not return after 5 seconds. This is a regression from the previous single `maybeCompact` call; please validate the trigger as strictly positive and add a regression test.
##########
paimon-common/src/main/java/org/apache/paimon/lookup/sort/db/LocalKvDb.java:
##########
@@ -549,7 +607,8 @@ private SstFileMetadata findFileForKey(List<SstFileMetadata> sortedFiles, Memory
private SstFileMetadata writeMemTableToSst(TreeMap<MemorySlice, byte[]> data)
throws IOException {
File sstFile = newSstFile();
- SortLookupStoreWriter writer = storeFactory.createWriter(sstFile, null);
+ SortLookupStoreWriter writer =
+ storeFactory.createWriter(sstFile, bloomFilterBuilderFactory.apply(data.size()));
Review Comment:
This is not an unchanged pre-existing limitation for this production path: `LocalKvDb` did not write Bloom filters before this PR, while this PR enables them by default for `ClusteringKeyIndex`, which uses `RowCompactedSerializer` comparator equality. Therefore the PR turns comparator-equal NaN encodings into false-negative lookups after flush. I think this should remain blocking unless the Bloom filter is disabled for such comparators or the hash input is made comparator-consistent.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Improve local KV database lookup performance [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #8857:
URL: https://github.com/apache/paimon/pull/8857#discussion_r3655817310
##########
paimon-common/src/main/java/org/apache/paimon/lookup/sort/db/LocalKvDb.java:
##########
@@ -302,6 +334,12 @@ public void bulkLoad(Iterator<Map.Entry<byte[], byte[]>> sortedEntries) throws I
previousFileMaxKey,
targetLevel);
}
+
+ checkArgument(
Review Comment:
Fixed in `a471fe2202`. On any `bulkLoad` validation, iterator, ordering, or I/O failure, the code now closes the in-progress writer, deletes `currentSstFile`, and deletes every SST recorded in `bulkLoadFiles` before rethrowing. The failure tests also assert that no SST files remain in the directory.
##########
paimon-common/src/main/java/org/apache/paimon/lookup/sort/db/AsyncLsmCompactor.java:
##########
@@ -0,0 +1,200 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.paimon.lookup.sort.db;
+
+import javax.annotation.Nullable;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.CancellationException;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+
+/** Coordinates asynchronous compaction, failure propagation, backpressure, and shutdown. */
+class AsyncLsmCompactor {
+
+ private final LsmLevels levels;
+ private final LsmCompactor compactor;
+ private final ExecutorService executor;
+ private final int levelZeroFileCountTrigger;
+ private final LsmCompactor.FileSupplier fileSupplier;
+ private final LsmCompactor.FileDeleter fileDeleter;
+ private final ThreadLocal<List<File>> deferredCompactionDeletes;
+
+ @Nullable private Future<?> compactionFuture;
+
+ AsyncLsmCompactor(
+ LsmLevels levels,
+ CompactorFactory compactorFactory,
+ ExecutorService executor,
+ int levelZeroFileCountTrigger,
+ LsmCompactor.FileSupplier fileSupplier,
+ LsmCompactor.FileDeleter fileDeleter) {
+ this.levels = levels;
+ this.executor = executor;
+ this.levelZeroFileCountTrigger = levelZeroFileCountTrigger;
+ this.fileSupplier = fileSupplier;
+ this.fileDeleter = fileDeleter;
+ this.deferredCompactionDeletes = new ThreadLocal<>();
+ this.compactor = compactorFactory.create(this::deferOrDeleteCompactedFile);
+ }
+
+ void scheduleIfNeeded() throws IOException {
+ checkFailure();
+ if (!needsCompaction() || compactionFuture != null) {
+ return;
+ }
+
+ try {
+ compactionFuture =
+ executor.submit(
+ () -> {
+ while (needsCompaction()) {
Review Comment:
Fixed in `71e293c8cc`. `LocalKvDb.Builder.level0FileNumCompactTrigger` now rejects zero and negative values before the database or asynchronous worker is created. Added `testLevel0FileNumCompactTriggerValidation` for both `0` and `-1`; the full `LocalKvDbTest` and `LocalKvDbAsyncCompactionTest` suites pass (59 tests).
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Improve local KV database lookup performance [paimon]
Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
leaves12138 commented on code in PR #8857:
URL: https://github.com/apache/paimon/pull/8857#discussion_r3655153671
##########
paimon-common/src/main/java/org/apache/paimon/lookup/sort/db/LocalKvDb.java:
##########
@@ -549,7 +607,8 @@ private SstFileMetadata findFileForKey(List<SstFileMetadata> sortedFiles, Memory
private SstFileMetadata writeMemTableToSst(TreeMap<MemorySlice, byte[]> data)
throws IOException {
File sstFile = newSstFile();
- SortLookupStoreWriter writer = storeFactory.createWriter(sstFile, null);
+ SortLookupStoreWriter writer =
+ storeFactory.createWriter(sstFile, bloomFilterBuilderFactory.apply(data.size()));
Review Comment:
Could we make the Bloom-filter hash consistent with the configured key comparator, or avoid enabling the Bloom filter when comparator equality is not guaranteed to imply byte equality?
`LocalKvDb` uses the configured comparator for MemTable/SST ordering and lookup, but the Bloom filter hashes the raw serialized key bytes. Thus two keys that compare equal but have different encodings can be found before flush and incorrectly rejected after flush.
This occurs with Paimon's own `RowCompactedSerializer` comparator: two FLOAT NaNs with different payload bits serialize to different bytes, while the slice comparator returns `0`. I reproduced:
rawKeysEqual=false, comparator=0
beforeFlush=value
afterFlush=null
Disabling the Bloom filter restores the expected result. Please add a regression test and either provide a comparator-consistent key hasher/canonical encoding or conservatively disable this optimization where byte equality is not guaranteed.
##########
paimon-common/src/main/java/org/apache/paimon/lookup/sort/db/LocalKvDb.java:
##########
@@ -302,6 +334,12 @@ public void bulkLoad(Iterator<Map.Entry<byte[], byte[]>> sortedEntries) throws I
previousFileMaxKey,
targetLevel);
}
+
+ checkArgument(
Review Comment:
Could we clean up all SST files created by `bulkLoad` when validation or iteration fails?
This validation can run after one or more output files have already been closed. For example, bulk-loading three entries with `numEntries=4` throws as expected, but leaves all three SST files in the directory while the database still reports zero level files:
sstFilesAfterFailure=3
dbLevelFilesAfterFailure=0
The same leak can occur when the iterator or an ordering check fails after earlier files have been finalized. Please delete `currentSstFile` and every file recorded in `bulkLoadFiles` before propagating the exception, and extend the failure tests to verify directory cleanup.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Improve LocalKvDb lookup and compaction performance [paimon]
Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
leaves12138 commented on code in PR #8857:
URL: https://github.com/apache/paimon/pull/8857#discussion_r3655916909
##########
paimon-common/src/main/java/org/apache/paimon/lookup/sort/db/AsyncLsmCompactor.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.paimon.lookup.sort.db;
+
+import javax.annotation.Nullable;
+
+import java.io.IOException;
+import java.util.concurrent.CancellationException;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+
+/** Coordinates asynchronous compaction, failure propagation, backpressure, and shutdown. */
+class AsyncLsmCompactor extends LsmCompactor {
+
+ private final ExecutorService executor;
+
+ @Nullable private Future<?> compactionFuture;
+
+ AsyncLsmCompactor(
+ LsmLevels levels,
+ CompactorFactory compactorFactory,
+ int levelZeroFileCountTrigger,
+ UniversalCompactor.FileSupplier fileSupplier,
+ UniversalCompactor.FileDeleter fileDeleter,
+ ExecutorService executor) {
+ super(levels, compactorFactory, levelZeroFileCountTrigger, fileSupplier, fileDeleter);
+ this.executor = executor;
+ }
+
+ @Override
+ void scheduleIfNeeded() throws IOException {
+ checkFailure();
+ if (!needsCompaction() || compactionFuture != null) {
Review Comment:
Could we coordinate worker termination and scheduling so a flush cannot miss compaction while the current `Future` is about to complete?
There is a race here:
1. The worker finishes a compaction round and its final `needsCompaction()` observes `false`.
2. Before that worker's `Future` completes, the foreground flush adds enough L0 files and calls `scheduleIfNeeded()`.
3. `needsCompaction()` is now `true`, but `compactionFuture != null`, so this method does not submit a new task.
4. The old worker then returns, leaving the newly flushed L0 files uncompacted until another write (and a close in the same window can miss it as well).
I reproduced this deterministically by pausing the worker after its final false stability check, adding a new L0 file, calling `scheduleIfNeeded()`, and then releasing the worker. After awaiting the old future, the L0 file count remains `1` and no task is pending. At the backpressure threshold, `applyBackpressure()` can similarly await the old future and return without scheduling the required compaction.
The running/pending state and the final stability check need a shared synchronization boundary, or an equivalent rescheduling handshake, rather than relying only on the non-null `Future` reference.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Improve local KV database lookup performance [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #8857:
URL: https://github.com/apache/paimon/pull/8857#discussion_r3655750291
##########
paimon-common/src/main/java/org/apache/paimon/lookup/sort/db/LocalKvDb.java:
##########
@@ -549,7 +607,8 @@ private SstFileMetadata findFileForKey(List<SstFileMetadata> sortedFiles, Memory
private SstFileMetadata writeMemTableToSst(TreeMap<MemorySlice, byte[]> data)
throws IOException {
File sstFile = newSstFile();
- SortLookupStoreWriter writer = storeFactory.createWriter(sstFile, null);
+ SortLookupStoreWriter writer =
+ storeFactory.createWriter(sstFile, bloomFilterBuilderFactory.apply(data.size()));
Review Comment:
`LocalTableQuery` already has the same limitation: it creates the sort lookup store with `RowCompactedSerializer.createSliceComparator()`, while its Bloom filter is also built and queried from the raw serialized key bytes (`lookup.cache.bloom.filter.enabled` is enabled by default). Therefore comparator-equal FLOAT/DOUBLE NaN payloads can likewise differ at the Bloom-filter level there. This is a broader existing limitation of custom-comparator equality versus raw-byte Bloom-filter hashing, rather than something specific to `LocalKvDb`. We will keep the behavior consistent in this PR and leave a general solution out of scope for now.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Improve LocalKvDb lookup and compaction performance [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #8857:
URL: https://github.com/apache/paimon/pull/8857
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Improve local KV database lookup performance [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #8857:
URL: https://github.com/apache/paimon/pull/8857#discussion_r3655467893
##########
paimon-common/src/main/java/org/apache/paimon/lookup/sort/db/LocalKvDb.java:
##########
@@ -549,7 +607,8 @@ private SstFileMetadata findFileForKey(List<SstFileMetadata> sortedFiles, Memory
private SstFileMetadata writeMemTableToSst(TreeMap<MemorySlice, byte[]> data)
throws IOException {
File sstFile = newSstFile();
- SortLookupStoreWriter writer = storeFactory.createWriter(sstFile, null);
+ SortLookupStoreWriter writer =
+ storeFactory.createWriter(sstFile, bloomFilterBuilderFactory.apply(data.size()));
Review Comment:
Thanks for pointing this out. This is a known limitation: the Bloom filter hashes the raw serialized key bytes, while a custom comparator can define broader equality, notably for different FLOAT/DOUBLE NaN payloads. We are leaving this limitation unchanged in this PR for now and will address comparator-consistent hashing or canonicalization separately.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Improve LocalKvDb lookup and compaction performance [paimon]
Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
leaves12138 commented on code in PR #8857:
URL: https://github.com/apache/paimon/pull/8857#discussion_r3655996774
##########
paimon-common/src/main/java/org/apache/paimon/lookup/sort/db/AsyncLsmCompactor.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.paimon.lookup.sort.db;
+
+import javax.annotation.Nullable;
+
+import java.io.IOException;
+import java.util.concurrent.CancellationException;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+
+/** Coordinates asynchronous compaction, failure propagation, backpressure, and shutdown. */
+class AsyncLsmCompactor extends LsmCompactor {
+
+ private final ExecutorService executor;
+
+ @Nullable private Future<?> compactionFuture;
+
+ AsyncLsmCompactor(
+ LsmLevels levels,
+ CompactorFactory compactorFactory,
+ int levelZeroFileCountTrigger,
+ UniversalCompactor.FileSupplier fileSupplier,
+ UniversalCompactor.FileDeleter fileDeleter,
+ ExecutorService executor) {
+ super(levels, compactorFactory, levelZeroFileCountTrigger, fileSupplier, fileDeleter);
+ this.executor = executor;
+ }
+
+ @Override
+ void scheduleIfNeeded() throws IOException {
+ checkFailure();
+ if (!needsCompaction() || compactionFuture != null) {
Review Comment:
After tracing the supported single-foreground-thread usage, the practical impact is much narrower than I stated above. Once the worker's final check sees fewer than `trigger` L0 files, only one foreground flush can occur before its own `scheduleIfNeeded()` call, so the count can reach at most `trigger`, not the `2 * trigger` backpressure threshold. Reads remain correct, and the next flush will clear the completed future and schedule compaction; if this happens during close, the local DB is ending its lifecycle anyway.
Therefore this is only a narrow delayed-compaction/performance case (up to the trigger-sized L0 set remaining until the next flush), not a correctness or backpressure issue. Please disregard the backpressure claim; I do not consider this blocking.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Improve LocalKvDb lookup and compaction performance [paimon]
Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
leaves12138 commented on code in PR #8857:
URL: https://github.com/apache/paimon/pull/8857#discussion_r3656274319
##########
paimon-common/src/main/java/org/apache/paimon/lookup/sort/db/LocalKvDb.java:
##########
@@ -670,20 +774,39 @@ public SimpleLsmKvDb build() {
}
if (cacheManager == null) {
- cacheManager = new CacheManager(MemorySize.ofMebiBytes(8));
+ cacheManager = new CacheManager(MemorySize.ofMebiBytes(8), 0);
}
SortLookupStoreFactory factory =
new SortLookupStoreFactory(
keyComparator, cacheManager, blockSize, compressOptions);
+ LongFunction<BloomFilter.Builder> bloomFilterBuilderFactory =
+ bloomFilterEnabled
+ ? expectedEntries ->
+ expectedEntries > 0
+ ? BloomFilter.fixedBuilder(
+ expectedEntries, bloomFilterFpp)
+ : BloomFilter.dynamicBuilder(bloomFilterFpp)
+ : expectedEntries -> null;
+ ExecutorService executor = null;
+ if (asyncCompact) {
+ executor =
+ compactionExecutor == null
+ ? Executors.newSingleThreadExecutor(
Review Comment:
Non-blocking scalability concern: every async `LocalKvDb` built without an injected executor creates its own single-thread executor. `ClusteringCompactManagerFactory` creates a manager (and therefore a `LocalKvDb`) per active partition/bucket, so after the first local compaction each DB retains one `local-kv-db-compaction` thread until that writer/DB is closed. A local lifecycle test with 24 DBs that each compacted left 24 such threads alive.
Could we use a dedicated shared bounded executor at the `FileStoreWrite`/factory scope, with executor ownership handled outside `LocalKvDb`, or otherwise cap/reuse these threads? I do not consider this a correctness blocker, but it may become significant for writers with many active partitions/buckets.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org