Skip to content

HDDS-11174. [hsync] Change XceiverClientRatis.watchForCommit to async.#6941

Merged
jojochuang merged 8 commits intoapache:masterfrom
szetszwo:HDDS-11174
Aug 7, 2024
Merged

HDDS-11174. [hsync] Change XceiverClientRatis.watchForCommit to async.#6941
jojochuang merged 8 commits intoapache:masterfrom
szetszwo:HDDS-11174

Conversation

@szetszwo
Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

In the code below, it should just return the replyFuture instead of calling CompletableFuture.get(). This change is useful for HDDS-9844.

//XceiverClientRatis.watchForCommit
      CompletableFuture<RaftClientReply> replyFuture = getClient().async().watch(index, watchType);
      final RaftClientReply reply = replyFuture.get();

What is the link to the Apache JIRA

HDDS-11174

How was this patch tested?

Updating existing tests.

@jojochuang jojochuang requested review from duongkame and smengcl July 16, 2024 20:04
@szetszwo
Copy link
Copy Markdown
Contributor Author

@duongkame and @smengcl , I will keep RatisBlockOutputStream unchanged here. Filed HDDS-11208 to do the further change. Please review this when you have time. Thanks.

@smengcl smengcl added the hbase HBase on Ozone support label Jul 22, 2024
@jojochuang
Copy link
Copy Markdown
Contributor

This PR won't make it into the HDDS-7593 feature branch before the branch gets merged next week.
Please rebase onto master branch after the branch merge. Thanks!

Copy link
Copy Markdown
Contributor

@jojochuang jojochuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. But the PR has a code conflict, and we're about to merge the feature branch into master, so please rebase against master after the branch is merged into master (should take at most another day).

Thanks!

@szetszwo
Copy link
Copy Markdown
Contributor Author

szetszwo commented Aug 6, 2024

Sure, will fix the conflicts after the merge.

@jojochuang jojochuang changed the base branch from HDDS-7593 to master August 6, 2024 17:54
public abstract XceiverClientReply watchForCommit(long index)
throws InterruptedException, ExecutionException, TimeoutException,
IOException;
public CompletableFuture<XceiverClientReply> watchForCommit(long index) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we also change this one to watchForCommitAsync without breaking compatibility?

public XceiverClientReply watchForCommit(long index)
throws InterruptedException, ExecutionException, TimeoutException,
IOException {
public CompletableFuture<XceiverClientReply> watchForCommit(long index) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we change this one to watchForCommitAsync without breaking compatibility?

Copy link
Copy Markdown
Contributor

@smengcl smengcl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @szetszwo . +1 other than the nits above.

@smengcl
Copy link
Copy Markdown
Contributor

smengcl commented Aug 6, 2024

Would you like to take a glance at this @duongkame ?

@jojochuang jojochuang merged commit 5561f21 into apache:master Aug 7, 2024
@jojochuang
Copy link
Copy Markdown
Contributor

Merged. Thanks @smengcl @szetszwo

@szetszwo
Copy link
Copy Markdown
Contributor Author

szetszwo commented Aug 7, 2024

@jojochuang , @smengcl , thanks for your helps!

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

Labels

hbase HBase on Ozone support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants