New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

PIP 76: Streaming Offload(Part I) #9096

Merged

codelipenghui merged 15 commits into apache:master from Renkai:new-offloader

Jan 26, 2021

Contributor

Renkai commented Dec 30, 2020 •

edited

Loading

This PR contains the new interface and implementation of the offloader in the below PIP

Unit test is still in progress

StreamingDataBlockHeaderImpl
StreamingBlobStoreBackedReadHandleImpl
BufferedOffloadStream
BlobStoreManagedLedgerOffloader
StreamingOffloadIndexBlock

PIP 76: https://github.com/apache/pulsar/wiki/PIP-76:-Streaming-Offload

sijie requested review from ivankelly, codelipenghui, jiazhai and merlimat

December 30, 2020 17:32

sijie assigned Renkai

sijie added the area/tieredstorage label

sijie added this to the 2.8.0 milestone

sijie changed the title ~~PIP : Streaming Offload~~ PIP 76: Streaming Offload

Member

sijie commented Jan 4, 2021

@Renkai I have assigned PIP-76 to this PIP and copied the content to the wiki page. https://github.com/apache/pulsar/wiki/PIP-76%3A-Streaming-Offload

Feel free to send the PIP to the dev@pulsar.apache.org mailing list.

sijie self-requested a review

January 4, 2021 17:39

Renkai marked this pull request as ready for review

January 6, 2021 05:18

Contributor Author

Renkai commented Jan 6, 2021

/pulsarbot run-failure-checks

Renkai changed the title ~~PIP 76: Streaming Offload~~ PIP 76: Streaming Offload(Part I)

Contributor Author

Renkai commented Jan 6, 2021

/pulsarbot run-failure-checks

codelipenghui requested changes

View reviewed changes

Contributor

codelipenghui left a comment •

edited

Loading

I left some comment, please check. And we have migrated to a new code generation lib at #9046, please rebase to the master branch and resolve the conflicts.

managed-ledger/src/main/proto/MLDataFormats.proto Outdated Show resolved Hide resolved

managed-ledger/src/main/proto/MLDataFormats.proto Outdated Show resolved Hide resolved

managed-ledger/src/main/proto/MLDataFormats.proto Outdated Show resolved Hide resolved

managed-ledger/src/main/proto/MLDataFormats.proto Outdated Show resolved Hide resolved

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/LedgerOffloader.java Outdated Show resolved Hide resolved

...n/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/TieredStorageConfiguration.java Outdated Show resolved Hide resolved

...n/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/TieredStorageConfiguration.java Show resolved Hide resolved

...d/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/StreamingOffloadIndexBlock.java Outdated Show resolved Hide resolved

...ain/java/org/apache/bookkeeper/mledger/offload/jcloud/StreamingOffloadIndexBlockBuilder.java Outdated Show resolved Hide resolved

...age/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/OffloadIndexEntry.java Outdated Show resolved Hide resolved

sijie requested changes

View reviewed changes

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/LedgerOffloader.java Outdated Show resolved Hide resolved

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/LedgerOffloader.java Show resolved Hide resolved

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/LedgerOffloader.java Outdated Show resolved Hide resolved

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedLedger.java Outdated Show resolved Hide resolved

...d/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/StreamingOffloadIndexBlock.java Outdated Show resolved Hide resolved

...a/org/apache/bookkeeper/mledger/offload/jcloud/impl/BlobStoreManagedLedgerOffloaderBase.java Show resolved Hide resolved

gaoran10 reviewed

View reviewed changes

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/LedgerOffloader.java Outdated Show resolved Hide resolved

managed-ledger/src/main/proto/MLDataFormats.proto Outdated Show resolved Hide resolved

managed-ledger/src/main/proto/MLDataFormats.proto Outdated Show resolved Hide resolved

...d/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/StreamingOffloadIndexBlock.java Outdated Show resolved Hide resolved

Renkai requested review from codelipenghui and sijie

January 12, 2021 10:25

eolivelli reviewed

View reviewed changes

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/LedgerOffloader.java Show resolved Hide resolved

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/LedgerOffloader.java Show resolved Hide resolved

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/LedgerOffloader.java Outdated Show resolved Hide resolved

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/LedgerOffloader.java Show resolved Hide resolved

.../java/org/apache/bookkeeper/mledger/offload/jcloud/impl/BlobStoreManagedLedgerOffloader.java Outdated Show resolved Hide resolved

.../java/org/apache/bookkeeper/mledger/offload/jcloud/impl/BlobStoreManagedLedgerOffloader.java Outdated Show resolved Hide resolved

.../java/org/apache/bookkeeper/mledger/offload/jcloud/impl/BlobStoreManagedLedgerOffloader.java Outdated Show resolved Hide resolved

Renkai force-pushed the new-offloader branch from 5ec03f7 to d3a6a8c Compare

January 13, 2021 09:02

Renkai mentioned this pull request

PIP 76: Streaming Offload(Part II) #9202

Closed

sijie reviewed

View reviewed changes

Member

sijie left a comment

@Renkai overall the change looks good to me. Left a couple of comments for clarifications.

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/LedgerOffloader.java Outdated Show resolved Hide resolved

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/LedgerOffloader.java Outdated Show resolved Hide resolved

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/LedgerOffloader.java Outdated Show resolved Hide resolved

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/LedgerOffloader.java Outdated Show resolved Hide resolved

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedLedger.java Outdated Show resolved Hide resolved

...a/org/apache/bookkeeper/mledger/offload/jcloud/impl/BlobStoreManagedLedgerOffloaderBase.java Show resolved Hide resolved

...rg/apache/bookkeeper/mledger/offload/jcloud/impl/StreamingBlobStoreBackedReadHandleImpl.java Outdated Show resolved Hide resolved

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/LedgerOffloader.java Outdated Show resolved Hide resolved

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/LedgerOffloader.java Outdated Show resolved Hide resolved

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/LedgerOffloader.java Show resolved Hide resolved

Renkai force-pushed the new-offloader branch from c708626 to bb0c55b Compare

January 15, 2021 09:17

Renkai requested a review from sijie

January 18, 2021 01:01

Contributor Author

Renkai commented Jan 18, 2021

@sijie PTAL, again

codelipenghui reviewed

View reviewed changes

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/LedgerOffloader.java Outdated Show resolved Hide resolved

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/LedgerOffloader.java Outdated Show resolved Hide resolved

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/LedgerOffloader.java Outdated Show resolved Hide resolved

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/LedgerOffloader.java Outdated Show resolved Hide resolved

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/LedgerOffloader.java Outdated Show resolved Hide resolved

.../java/org/apache/bookkeeper/mledger/offload/jcloud/impl/BlobStoreManagedLedgerOffloader.java Outdated Show resolved Hide resolved

...d/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/impl/BufferedOffloadStream.java Outdated Show resolved Hide resolved

...rg/apache/bookkeeper/mledger/offload/jcloud/impl/StreamingBlobStoreBackedReadHandleImpl.java Outdated Show resolved Hide resolved

...rg/apache/bookkeeper/mledger/offload/jcloud/impl/StreamingBlobStoreBackedReadHandleImpl.java Outdated Show resolved Hide resolved

...rg/apache/bookkeeper/mledger/offload/jcloud/impl/StreamingBlobStoreBackedReadHandleImpl.java Outdated

+                              return;
+                          }
+                          for (GroupedReader groupedReader : groupedReaders) {

Contributor

codelipenghui Jan 19, 2021

Please double confirm the groupedReaders are sorted by ledger Id, entry id, Otherwise, this will bread the entry read in order guarantee

Contributor Author

Renkai Jan 19, 2021

double confirm code added

Renkai requested a review from codelipenghui

January 19, 2021 15:15


          snapshot 20210120

45d435c

Signed-off-by: Renkai <gaelookair@gmail.com>

Renkai force-pushed the new-offloader branch from 44109dc to 45d435c Compare

January 20, 2021 02:54

Renkai added 2 commits

January 20, 2021 14:20


          fix bug to pass tests

8f4510a

Signed-off-by: Renkai <gaelookair@gmail.com>


          merge the from stream method of two version index

b8b53dc

Signed-off-by: Renkai <gaelookair@gmail.com>

Renkai force-pushed the new-offloader branch from 4e4d766 to b8b53dc Compare

January 20, 2021 07:51


          add info to prevent offload multi times

26cdd20

Signed-off-by: Renkai <gaelookair@gmail.com>

Contributor Author

Renkai commented Jan 20, 2021

/pulsarbot run-failure-checks

Renkai added 2 commits

January 20, 2021 21:26


          reduce sleep time

ce767ae

Signed-off-by: Renkai <gaelookair@gmail.com>


          put blob after buffer reaches threshold

6ecde44

Signed-off-by: Renkai <gaelookair@gmail.com>

Contributor Author

Renkai commented Jan 21, 2021

/pulsarbot run-failure-checks

1 similar comment

Contributor Author

Renkai commented Jan 21, 2021

/pulsarbot run-failure-checks


          Merge branch 'master' into new-offloader

7794e18

codelipenghui reviewed

View reviewed changes

Contributor

codelipenghui left a comment

The change looks better now, just left some minor comments.

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java

+                  @Override
+                  public CompletableFuture<LedgerInfo> getLedgerInfo(long ledgerId) {
+                      CompletableFuture<LedgerInfo> result = new CompletableFuture<>();
+                      final LedgerInfo ledgerInfo = ledgers.get(ledgerId);

Contributor

codelipenghui Jan 23, 2021

And please consider returns a copy of LedgerInfo to void be modified from external?

...ain/java/org/apache/bookkeeper/mledger/offload/jcloud/StreamingOffloadIndexBlockBuilder.java Outdated Show resolved Hide resolved

...n/java/org/apache/bookkeeper/mledger/offload/jcloud/impl/StreamingOffloadIndexBlockImpl.java Outdated Show resolved Hide resolved

...ain/java/org/apache/bookkeeper/mledger/offload/jcloud/impl/StreamingDataBlockHeaderImpl.java

+              /**
+               * The data block header in code storage for each data block.
+               */
+              public class StreamingDataBlockHeaderImpl implements DataBlockHeader {

Contributor

codelipenghui Jan 23, 2021

Have you checked this comment @Renkai?

Renkai added 2 commits

January 24, 2021 22:23


          Merge branch 'master' into new-offloader

82c6cb1


          polish

c622056

Signed-off-by: Renkai <gaelookair@gmail.com>

Renkai requested a review from codelipenghui

January 24, 2021 14:36

Contributor Author

Renkai commented Jan 25, 2021

/pulsarbot run-failure-checks

Renkai added 2 commits

January 25, 2021 09:01


          change streaming to v2

9e9b1c6

Signed-off-by: Renkai <gaelookair@gmail.com>


          rename streaming to v2

ea69a1d

Signed-off-by: Renkai <gaelookair@gmail.com>

Contributor Author

Renkai commented Jan 25, 2021

/pulsarbot run-failure-checks

2 similar comments

Contributor Author

Renkai commented Jan 25, 2021

/pulsarbot run-failure-checks

Contributor Author

Renkai commented Jan 25, 2021

/pulsarbot run-failure-checks

codelipenghui approved these changes

View reviewed changes

sijie approved these changes

View reviewed changes

Contributor Author

Renkai commented Jan 25, 2021

/pulsarbot run-failure-checks

1 similar comment

Contributor Author

Renkai commented Jan 25, 2021

/pulsarbot run-failure-checks


          Merge branch 'master' into new-offloader

e4835d4

Contributor Author

Renkai commented Jan 26, 2021

/pulsarbot run-failure-checks

3 similar comments

Contributor Author

Renkai commented Jan 26, 2021

/pulsarbot run-failure-checks

Contributor Author

Renkai commented Jan 26, 2021

/pulsarbot run-failure-checks

Contributor Author

Renkai commented Jan 26, 2021

/pulsarbot run-failure-checks

codelipenghui merged commit 17f399e into apache:master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

eolivelli eolivelli left review comments

gaoran10 gaoran10 left review comments

zymap zymap left review comments

sijie sijie approved these changes

codelipenghui codelipenghui approved these changes

ivankelly Awaiting requested review from ivankelly

jiazhai Awaiting requested review from jiazhai

merlimat Awaiting requested review from merlimat

Labels

area/tieredstorage