[BP-62] Refactor read op, and introduce batchReadOp. #4190
Conversation
hangc0276
requested review from
merlimat,
hangc0276,
dlg99,
eolivelli,
zymap,
wenbingshen and
StevenLuMT
| this.requestTimeNanos = MathUtils.nowInNano(); | ||
| List<BookieId> ensemble = getLedgerMetadata().getEnsembleAt(startEntryId); | ||
| if (parallelRead) { | ||
| LOG.info("Batch read unsupported the parallelRead, fail back to sequence read."); |
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| public class BatchedReadOp extends ReadOpBase implements BatchedReadEntryCallback { |
There was a problem hiding this comment.
We'd better add a test for BatchedReadOp and PendingReadOp to cover this change.
There was a problem hiding this comment.
The class TestParallelRead already covers the PendingReadOp. And I will cover the BatchedReadOp at the next PR. At the next PR, I will introduce the batch read API in LedgerHandle.
There was a problem hiding this comment.
Do we need a unit test to ensure the bytebuf ref count is correct and the request is handled correctly, like how the ParallelRead test did?
There was a problem hiding this comment.
Do we need a unit test to ensure the bytebuf ref count is correct and the request is handled correctly, like how the ParallelRead test did?
That makes sense. We would better rely on the LedgerHandle to test it. This PR just refactors the code, we should ensure the PendingReadOp works as well as before.
At the next PR, I will introduce tests to cover BatchedReadOP.
|
@merlimat @lhotari @eolivelli Please help take a look, thanks. |
|
rerun failure checks |
### Motivation This is the fourth PR for the batch read(apache#4051) feature. Refactor read op, extract ReadOpBase. Introduce batchedReadOp.
### Motivation This is the fifth PR for the batch read(apache#4051) feature. LedgerHandle introduces batch read API. This PR is based on apache#4190, please merge it firstly.
Motivation
This is the fourth PR for the batch read(#4051) feature.
Refactor read op, extract ReadOpBase. Introduce batchedReadOp.