KAFKA-12368: Added inmemory implementations for RemoteStorageManager and RemoteLogMetadataManager. by satishd · Pull Request #10218 · apache/kafka

satishd · 2021-02-26T08:47:53Z

KAFKA-12368: Added inmemory implementations for RemoteStorageManager and RemoteLogMetadataManager.

Added inmemory implementation for RemoteStorageManager and RemoteLogMetadataManager. A major part of inmemory RLMM will be used in the default RLMM implementation which will be based on topic storage. These will be used in unit tests for tiered storage.
Added tests for both the implementations and their supported classes.

This is part of tiered storage implementation, KIP-405.

Committer Checklist (excluded from commit message)

Verify design and implementation
Verify test coverage and CI build status
Verify documentation (including upgrade notes)

junrao

@satishd : Thanks for the PR. A couple of quick comments.

junrao

@satishd : Thanks for the PR. A few more comments below.

kowshik

@satishd Thanks for the PR! Few comments below.

satishd · 2021-03-12T04:56:35Z

thanks @junrao @kowshik for the review comments, addressed with the latest commits/comments.

junrao

@satishd : Thanks for the updated PR. A few more comments below.

kowshik

@satishd Thanks for the updated PR! Few more comments below.

kowshik

@satishd Thanks for the updates! Few more comments below, this time I made a pass on some of the test code as well.

kowshik · 2021-03-22T19:52:27Z

Hmm, do we need to explicitly check if endPosition < segment.length?

It takes Math.min(endPosition, segment.length). So, no need to have that check.

satishd · 2021-03-25T04:45:09Z

@kowshik I discussed the proposed changes in the call on 23rd, and I mentioned that PR is not updated with those changes. I will let you know once those changes are pushed into this PR.

satishd · 2021-04-02T15:14:03Z

@junrao @kowshik thanks for your review and comments. In the latest commit, addressed the review comments. I have also refactored the code with better abstractions.

junrao

@satishd : Thanks for the updated PR. A few more comments below.

satishd · 2021-04-06T07:32:23Z

Thanks @junrao for the review. Addressed them with the latest commit.

satishd · 2021-04-07T16:44:27Z

@junrao This PR is on top of #10489 . So, #10489 be merged before merging this PR.

junrao

@satishd : Thanks for the updated PR. A few more comments below.

junrao · 2021-04-07T22:14:31Z

It's kind of weird that the segment with epoch 0 is already deleted and yet we still expect the highest offset for epoch 0 to be returned.

highestLogOffset can contain the deleted segments. highestLogOffset means the highest offset up to which the segments have been copied. Pl take a look at the comment.

…and RemoteLogMetadataManager. Added inmemory implementation for RemoteStorageManager and RemoteLogMetadataManager. A major part of inmemory RLMM will be used in the default RLMM implementation which will be based on topic storage. These will be used in unit tests for tiered storage. Added a few tests for both the implementations. This is part of tiered storage implementation, KIP-405.

Updated java doc comments and addressed typos.

Refactoring for better modularization. Addressed review comments.

satishd · 2021-04-08T07:14:14Z

@junrao Thanks for your comments. Replied on comments and addressed them with the b5c0081 commit.

junrao

@satishd : Thanks for the updated PR. A few more comments.

kowshik

@satishd Thanks for the updated PR! Few more comments.

satishd · 2021-04-09T14:47:16Z

Thanks @junrao @kowshik for the comments. Addressed them with the d12c77a.

junrao

@satishd : Thanks for the updated PR. Just a minor comment below.

satishd · 2021-04-09T23:09:05Z

Thanks @junrao for the comment, addressed with the commit 317be7a.

junrao

@satishd : Thanks for the updated PR. LGTM. I will see if Kowshik has any further comments.

kowshik

@satishd: Thanks for the updated PR. Just a minor comment below. Let us wait for CI to pass before merge.

kowshik · 2021-04-12T00:13:25Z

+ *  The below table summarizes whether the segment with the respective state are available for the given methods.
+ * <pre>
+ * +---------------------------------+----------------------+------------------------+-------------------------+-------------------------+
+ * |  Method / SegmentState          | COPY_SEGMENT_STARTED | COPY_SEGMENT_FINISHED  | DELETE_SEGMENT_STARTED  | DELETE_SEGMENT_STARTED  |


typo: The title of the last column should be DELETE_SEGMENT_FINISHED.

thanks, addressed it in the latest commit.

satishd · 2021-04-12T03:24:39Z

@junrao @kowshik It looks like failures are not related to this PR.

…and RemoteLogMetadataManager. (apache#10218) KAFKA-12368: Added inmemory implementations for RemoteStorageManager and RemoteLogMetadataManager. Added inmemory implementation for RemoteStorageManager and RemoteLogMetadataManager. A major part of inmemory RLMM will be used in the default RLMM implementation which will be based on topic storage. These will be used in unit tests for tiered storage. Added tests for both the implementations and their supported classes. This is part of tiered storage implementation, KIP-405. Reivewers: Kowshik Prakasam <kprakasam@confluent.io>, Jun Rao <junrao@gmail.com>

…and RemoteLogMetadataManager. (apache#10218) Summary: KAFKA-12368: Added inmemory implementations for RemoteStorageManager and RemoteLogMetadataManager. Added inmemory implementation for RemoteStorageManager and RemoteLogMetadataManager. A major part of inmemory RLMM will be used in the default RLMM implementation which will be based on topic storage. These will be used in unit tests for tiered storage. Added tests for both the implementations and their supported classes. This is part of tiered storage implementation, KIP-405. apache-reviewers: Kowshik Prakasam <kprakasam@confluent.io>, Jun Rao <junrao@gmail.com> (cherry picked from commit 3278090) Reviewers: #ldap_kafka_admins, kchandraprakash Reviewed By: #ldap_kafka_admins, kchandraprakash JIRA Issues: DKAFC-868 Differential Revision: https://code.uberinternal.com/D6303209

satishd force-pushed the KAFKA-12368 branch from 104410b to cccbe73 Compare March 1, 2021 07:39

satishd marked this pull request as draft March 1, 2021 15:31

satishd force-pushed the KAFKA-12368 branch 7 times, most recently from df6b7b3 to 2f0c2f1 Compare March 4, 2021 14:26

satishd marked this pull request as ready for review March 4, 2021 14:26

satishd force-pushed the KAFKA-12368 branch 2 times, most recently from 74f273b to a03cb96 Compare March 5, 2021 11:57

satishd mentioned this pull request Mar 5, 2021

KAFKA-12429: Added serdes for the default implementation of RLMM based on an internal topic as storage. #10271

Merged

3 tasks

junrao reviewed Mar 9, 2021

View reviewed changes

Comment thread ...c/main/java/org/apache/kafka/server/log/remote/storage/InmemoryRemoteLogMetadataManager.java Outdated

Comment thread ...s/src/main/java/org/apache/kafka/server/log/remote/storage/InmemoryRemoteStorageManager.java Outdated

junrao reviewed Mar 10, 2021

View reviewed changes

satishd force-pushed the KAFKA-12368 branch from f1c3612 to b62cff0 Compare March 10, 2021 16:55

kowshik reviewed Mar 11, 2021

View reviewed changes

satishd force-pushed the KAFKA-12368 branch 3 times, most recently from b61336f to 0ce29f8 Compare March 12, 2021 04:55

junrao reviewed Mar 15, 2021

View reviewed changes

kowshik reviewed Mar 16, 2021

View reviewed changes

kowshik reviewed Mar 22, 2021

View reviewed changes

satishd force-pushed the KAFKA-12368 branch 2 times, most recently from dfa913c to 96d05f9 Compare April 2, 2021 15:11

junrao reviewed Apr 5, 2021

View reviewed changes

satishd force-pushed the KAFKA-12368 branch 2 times, most recently from 0711931 to 3f88549 Compare April 7, 2021 14:21

junrao reviewed Apr 7, 2021

View reviewed changes

satishd added 5 commits April 8, 2021 07:52

Moved inmemory implementations to a separate module.

d3c86ee

Updated java doc comments and addressed typos.

Extracted remote log leader epoch from cache into a separate class.

cb9b81a

Refactoring for better modularization. Addressed review comments.

Addressed review comments.

2571ec0

Renamed 'remote-storage' module to 'storage' module.

7315edf

satishd force-pushed the KAFKA-12368 branch from 3f88549 to 7315edf Compare April 8, 2021 02:28

Addressed review comments

b5c0081

junrao reviewed Apr 8, 2021

View reviewed changes

kowshik reviewed Apr 9, 2021

View reviewed changes

Addressed review comments.

d12c77a

junrao reviewed Apr 9, 2021

View reviewed changes

Comment thread ...rc/main/java/org/apache/kafka/server/log/remote/metadata/storage/RemoteLogMetadataCache.java Outdated

Fixed a minor typo.

317be7a

junrao approved these changes Apr 11, 2021

View reviewed changes

kowshik approved these changes Apr 12, 2021

View reviewed changes

Addressed a minor typo.

f854442

junrao merged commit 3278090 into apache:trunk Apr 13, 2021

kamalcph mentioned this pull request May 9, 2024

KAFKA-16696: Removed the in-memory implementation of RSM and RLMM #15911

Merged

3 tasks

Conversation

satishd commented Feb 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Committer Checklist (excluded from commit message)

Uh oh!

junrao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

junrao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kowshik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

satishd commented Mar 12, 2021

Uh oh!

junrao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kowshik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kowshik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kowshik Mar 22, 2021

Choose a reason for hiding this comment

Uh oh!

satishd Apr 2, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

satishd commented Mar 25, 2021

Uh oh!

satishd commented Apr 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

junrao left a comment

satishd commented Feb 26, 2021 •

edited

Loading

satishd commented Apr 2, 2021 •

edited

Loading