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/05/28 01:51:18 UTC
[PR] [global-index] Refactor index read path to async with CompletableFuture [paimon]
JingsongLi opened a new pull request, #8003:
URL: https://github.com/apache/paimon/pull/8003
Convert GlobalIndexReader from synchronous to asynchronous by returning CompletableFuture<Optional<GlobalIndexResult>>, enabling concurrent I/O across multiple index files. Remove lazy evaluation from GlobalIndexResult in favor of eager computation after async completion. Add ExecutorService to BTreeIndexReader and default global-index.thread-num to 32.
Remove unused BitmapGlobalIndex and FileIndexReaderWrapper/WriterWrapper. Add BTreeThreadSafetyTest for concurrent access validation. Mirror changes in the Python SDK with new lazy_filtered_btree_reader and btree_file_meta_selector modules.
--
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] [global-index] Refactor index read path to async with CompletableFuture [paimon]
Posted by "XiaoHongbo-Hope (via GitHub)" <gi...@apache.org>.
XiaoHongbo-Hope commented on code in PR #8003:
URL: https://github.com/apache/paimon/pull/8003#discussion_r3316219860
##########
paimon-python/pypaimon/globalindex/tantivy/tantivy_full_text_global_index_reader.py:
##########
@@ -164,37 +163,38 @@ def visit_full_text_search(self, full_text_search) -> Optional[ScoredGlobalIndex
for row_id, score in zip(row_ids, scores):
id_to_scores[row_id] = score
- return DictBasedScoredIndexResult(id_to_scores)
+ return _completed_future(DictBasedScoredIndexResult(id_to_scores))
Review Comment:
The synchronous `searcher.search(...)` result wrapped in
`_completed_future`
makes `wait(futures)` in `full_text_read.py` a no-op — same shape in
`lumina_vector_global_index_reader.py`.
--
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] [global-index] Refactor index read path to async with CompletableFuture [paimon]
Posted by "XiaoHongbo-Hope (via GitHub)" <gi...@apache.org>.
XiaoHongbo-Hope commented on code in PR #8003:
URL: https://github.com/apache/paimon/pull/8003#discussion_r3316408006
##########
paimon-python/pypaimon/globalindex/tantivy/tantivy_full_text_global_index_reader.py:
##########
@@ -164,37 +163,38 @@ def visit_full_text_search(self, full_text_search) -> Optional[ScoredGlobalIndex
for row_id, score in zip(row_ids, scores):
id_to_scores[row_id] = score
- return DictBasedScoredIndexResult(id_to_scores)
+ return _completed_future(DictBasedScoredIndexResult(id_to_scores))
Review Comment:
>
Git it
--
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] [global-index] Refactor index read path to async with CompletableFuture [paimon]
Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
leaves12138 commented on PR #8003:
URL: https://github.com/apache/paimon/pull/8003#issuecomment-4561053851
I did another thread-safety focused pass on the latest revision (`a85568af6e`). The mutable `KeySerializer` buffer issue looks fixed now, and the BTree reader path mostly looks safe: each range query creates its own `SstFileIterator` / `BlockIterator`, `BlockCache` uses a concurrent map and synchronizes non-vectored stream access, and `LazyField` uses double-checked locking for the null bitmap.
However, I found one remaining concurrency/liveness issue that I think should be fixed before merging.
`LazyFilteredBTreeReader.visitParallel` nests async submissions onto the same executor:
```java
CompletableFuture.supplyAsync(() -> getOrCreateReader(meta), executor)
.thenCompose(visitor)
```
and each `visitor` call eventually enters `BTreeIndexReader.supplyResult`, which does another `CompletableFuture.supplyAsync(..., executor)`.
This is risky with the executor returned by `GlobalIndexReadThreadPool.getExecutorService(threadNum)`. When `threadNum` is smaller than the shared pool max, it returns a `SemaphoredDelegatingExecutor`, whose `execute/submit` acquires permits synchronously. With `global-index.thread-num = 1`, the first stage may still be holding the only permit when the `thenCompose` callback tries to submit the second stage to the same executor, so the worker thread blocks acquiring a permit that it can only release after the callback returns. That can deadlock the query.
A minimal reproduction of the executor pattern is:
```java
ExecutorService base = ThreadPoolUtils.createCachedThreadPool(4, "demo");
ExecutorService executor = new SemaphoredDelegatingExecutor(base, 1, false);
CountDownLatch latch = new CountDownLatch(1);
CompletableFuture<String> outer = CompletableFuture.supplyAsync(() -> {
try {
latch.await();
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
return "outer";
}, executor);
CompletableFuture<String> f = outer.thenCompose(
v -> CompletableFuture.supplyAsync(() -> "inner", executor));
latch.countDown();
f.get(2, TimeUnit.SECONDS); // TimeoutException
```
I also ran the current BTree concurrency tests locally and they pass:
```bash
mvn -pl paimon-common -Pfast-build -Dtest=BTreeThreadSafetyTest,LazyFilteredBTreeIndexReaderTest test
```
but those tests use regular fixed thread pools / direct executors, so they do not cover the `SemaphoredDelegatingExecutor` path used when users configure a smaller `global-index.thread-num`.
Possible fixes:
1. Avoid the nested async hop in `LazyFilteredBTreeReader`, e.g. create/get the reader synchronously and call the visitor directly in a single `supplyAsync` stage, or
2. make `BTreeIndexReader` expose synchronous read methods internally so `LazyFilteredBTreeReader` owns exactly one async submission per selected file, or
3. add a test using `GlobalIndexReadThreadPool.getExecutorService(1)` / `SemaphoredDelegatingExecutor` to prevent regressions.
--
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] [global-index] Refactor index read path to async with CompletableFuture [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #8003:
URL: https://github.com/apache/paimon/pull/8003#discussion_r3316327648
##########
paimon-python/pypaimon/globalindex/tantivy/tantivy_full_text_global_index_reader.py:
##########
@@ -164,37 +163,38 @@ def visit_full_text_search(self, full_text_search) -> Optional[ScoredGlobalIndex
for row_id, score in zip(row_ids, scores):
id_to_scores[row_id] = score
- return DictBasedScoredIndexResult(id_to_scores)
+ return _completed_future(DictBasedScoredIndexResult(id_to_scores))
Review Comment:
It is indeed a reasonable design: Python GIL restricts true parallelism, while the asynchronous interface maintains the same API form as the Java side, which can be replaced with asyncio or thread pool implementation in the future
--
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] [global-index] Refactor index read path to async with CompletableFuture [paimon]
Posted by "XiaoHongbo-Hope (via GitHub)" <gi...@apache.org>.
XiaoHongbo-Hope commented on PR #8003:
URL: https://github.com/apache/paimon/pull/8003#issuecomment-4564406964
+1
--
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] [global-index] Refactor index read path to async with CompletableFuture [paimon]
Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
leaves12138 commented on PR #8003:
URL: https://github.com/apache/paimon/pull/8003#issuecomment-4560450294
Thanks for the update. The previous compilation blockers look fixed to me now:
- `paimon-lumina` test compilation passes locally with:
```bash
mvn -pl paimon-lumina -am -Pfast-build -DskipTests test-compile
```
- The targeted common tests still pass:
```bash
mvn -pl paimon-common -Pfast-build -Dtest=GlobalIndexEvaluatorTest,BTreeThreadSafetyTest,LazyFilteredBTreeIndexReaderTest,BTreeIndexReaderTest,GlobalIndexSerDeUtilsTest test
```
I found one remaining consistency issue around the bitmap removal: the procedure docs still mention bitmap global index support even though the bitmap global index implementation and service registration have been removed.
Please update these stale references:
- `docs/docs/spark/procedures.md`: `create_global_index` / `drop_global_index` still say `btree` or `bitmap`.
- `docs/docs/flink/procedures.md`: `create_global_index` / `drop_global_index` still list bitmap examples and supported types including `bitmap`.
- Minor comments also still mention bitmap in `FileType` docs (`FileType.java` and Python `file_type.py`).
After the docs are aligned with the implementation, this looks good to me.
--
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] [global-index] Refactor index read path to async with CompletableFuture [paimon]
Posted by "XiaoHongbo-Hope (via GitHub)" <gi...@apache.org>.
XiaoHongbo-Hope commented on code in PR #8003:
URL: https://github.com/apache/paimon/pull/8003#discussion_r3316219860
##########
paimon-python/pypaimon/globalindex/tantivy/tantivy_full_text_global_index_reader.py:
##########
@@ -164,37 +163,38 @@ def visit_full_text_search(self, full_text_search) -> Optional[ScoredGlobalIndex
for row_id, score in zip(row_ids, scores):
id_to_scores[row_id] = score
- return DictBasedScoredIndexResult(id_to_scores)
+ return _completed_future(DictBasedScoredIndexResult(id_to_scores))
Review Comment:
The synchronous `searcher.search(...)` result wrapped in
`_completed_future`
makes `wait(futures)` in `full_text_read.py` a no-op — same shape in
`lumina_vector_global_index_reader.py`. Is it by design?
--
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] [global-index] Refactor index read path to async with CompletableFuture [paimon]
Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
leaves12138 commented on PR #8003:
URL: https://github.com/apache/paimon/pull/8003#issuecomment-4560304567
Thanks for the refactor. I don't think this PR is ready to approve yet because there are a few blockers/inconsistencies to address first.
1. **`paimon-lumina` test compilation fails.**
`LuminaVectorGlobalIndexReader` now requires an `ExecutorService` constructor argument, but some test/benchmark code still uses the old constructor signature. I hit this with:
```bash
mvn -pl paimon-spark/paimon-spark-ut -am -Pfast-build -DskipTests test-compile
```
The failures are in:
- `paimon-lumina/src/test/java/org/apache/paimon/lumina/index/LuminaVectorGlobalIndexTest.java`
- `paimon-lumina/src/test/java/org/apache/paimon/lumina/index/LuminaVectorBenchmark.java`
2. **Bitmap global index is removed, but Spark procedure tests still expect it.**
This PR removes the bitmap global index implementation and its service registration, leaving only `BTreeGlobalIndexerFactory` in `META-INF/services/org.apache.paimon.globalindex.GlobalIndexerFactory`. However, `CreateGlobalIndexProcedureTest` still contains several `index_type => 'bitmap'` cases. If bitmap global index is intentionally removed, please also update/remove those tests and mention the compatibility impact in the PR description/docs. Otherwise, the bitmap factory should be kept.
Minor follow-ups:
- `global-index.thread-num` changes from the previous default of available processors to a fixed default of `32`; please confirm this behavior change is intended.
- `GlobalIndexEvaluator` still accepts an `ExecutorService` constructor argument but no longer uses it.
- `VectorReadImpl` and `FullTextReadImpl` pass `newDirectExecutorService()` to per-split readers, so the async path may not provide cross-split parallelism there. If cross-split parallelism is still desired, consider reusing the configured global-index executor.
For reference, these targeted common tests passed locally:
```bash
mvn -pl paimon-common -Pfast-build -Dtest=GlobalIndexEvaluatorTest,BTreeThreadSafetyTest,LazyFilteredBTreeIndexReaderTest,BTreeIndexReaderTest,GlobalIndexSerDeUtilsTest test
```
--
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] [global-index] Refactor index read path to async with CompletableFuture [paimon]
Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
leaves12138 merged PR #8003:
URL: https://github.com/apache/paimon/pull/8003
--
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