Lucene subproject calls CompletableFuture::join #1571
Assignees
Labels
bug
Something isn't working
Comments
alecgrieser
added a commit
to alecgrieser/fdb-record-layer
that referenced
this issue
Mar 29, 2022
This removes the calls to `CompletableFuture::join` from non-test code within the fdb-record-layer-lucene subproject and replaces them with calls to `FDBRecordContext::asyncToSync` over the future. This also made a few code improvements that I noticed while I was editing things: 1. There were a few remaining calls to `.thenApplyAsync`, which shuffles work onto the common fork join pool, and therefore doesn't work with the Record Layer threading model 1. The `readBlock` method could be pipelined better so that it didn't block on getting the file reference, but instead composed the file-reference future and the future to read the block 1. The parameteried `FDBLuceneQueryTest` tests were hiding their names
alecgrieser
added a commit
to alecgrieser/fdb-record-layer
that referenced
this issue
Mar 29, 2022
This removes the calls to `CompletableFuture::join` from non-test code within the fdb-record-layer-lucene subproject and replaces them with calls to `FDBRecordContext::asyncToSync` over the future. This also made a few code improvements that I noticed while I was editing things: 1. There were a few remaining calls to `.thenApplyAsync`, which shuffles work onto the common fork join pool, and therefore doesn't work with the Record Layer threading model 1. The `readBlock` method could be pipelined better so that it didn't block on getting the file reference, but instead composed the file-reference future and the future to read the block 1. The parameteried `FDBLuceneQueryTest` tests were hiding their names
alecgrieser
added a commit
to alecgrieser/fdb-record-layer
that referenced
this issue
Mar 29, 2022
This removes the calls to `CompletableFuture::join` from non-test code within the fdb-record-layer-lucene subproject and replaces them with calls to `FDBRecordContext::asyncToSync` over the future. This also made a few code improvements that I noticed while I was editing things: 1. There were a few remaining calls to `.thenApplyAsync`, which shuffles work onto the common fork join pool, and therefore doesn't work with the Record Layer threading model 1. The `readBlock` method could be pipelined better so that it didn't block on getting the file reference, but instead composed the file-reference future and the future to read the block 1. The parameteried `FDBLuceneQueryTest` tests were hiding their names
alecgrieser
added a commit
to alecgrieser/fdb-record-layer
that referenced
this issue
Apr 4, 2022
This removes the calls to `CompletableFuture::join` from non-test code within the fdb-record-layer-lucene subproject and replaces them with calls to `FDBRecordContext::asyncToSync` over the future. This also made a few code improvements that I noticed while I was editing things: 1. There were a few remaining calls to `.thenApplyAsync`, which shuffles work onto the common fork join pool, and therefore doesn't work with the Record Layer threading model 1. The `readBlock` method could be pipelined better so that it didn't block on getting the file reference, but instead composed the file-reference future and the future to read the block 1. The parameteried `FDBLuceneQueryTest` tests were hiding their names
alecgrieser
added a commit
that referenced
this issue
Apr 5, 2022
This removes the calls to `CompletableFuture::join` from non-test code within the fdb-record-layer-lucene subproject and replaces them with calls to `FDBRecordContext::asyncToSync` over the future. This also made a few code improvements that I noticed while I was editing things: 1. There were a few remaining calls to `.thenApplyAsync`, which shuffles work onto the common fork join pool, and therefore doesn't work with the Record Layer threading model 1. The `readBlock` method could be pipelined better so that it didn't block on getting the file reference, but instead composed the file-reference future and the future to read the block 1. The parameteried `FDBLuceneQueryTest` tests were hiding their names
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
There are a few places in the Lucene subproject implementation that I could find calls to
CompletableFuture::joinorCompletableFuture::get. In non-test code, that's generally a bad sign for a few different reasons:It doesn't run the exception wrapping code, so it can expose exceptions from the underlying system instead of, say,
RecordCoreExceptionsTimeouts aren't controlled by the timeout handler on the database
Blocking-in-async detection is always skipped (which may need to be handled a little more delicately for Lucene because of the threading model in play)
We should replace those instances with calls to
FDBRecordContext::asyncToSyncThe text was updated successfully, but these errors were encountered: