Skip to content

HDDS-10039. Remove the flushLatches set from FlushNotifier.#5900

Merged
adoroszlai merged 2 commits intoapache:masterfrom
szetszwo:HDDS-10039
Jan 10, 2024
Merged

HDDS-10039. Remove the flushLatches set from FlushNotifier.#5900
adoroszlai merged 2 commits intoapache:masterfrom
szetszwo:HDDS-10039

Conversation

@szetszwo
Copy link
Contributor

@szetszwo szetszwo commented Jan 1, 2024

What changes were proposed in this pull request?

In FlushNotifier, it add a new latch to the flushLatches set for each await() call.

We may reduce the number of objects by using CompletableFuture. Then, the await() calls waiting for the same flush will use for the same future.

What is the link to the Apache JIRA

HDDS-10039

How was this patch tested?

By updating existing tests.

@smengcl smengcl requested a review from GeorgeJahad January 4, 2024 08:19
@GeorgeJahad
Copy link
Contributor

smengcl requested a review from GeorgeJahad

@smengcl I'm on vacation and won't have time to look at this till I get back, (in 2 weeks.)

@szetszwo
Copy link
Contributor Author

szetszwo commented Jan 8, 2024

@smengcl , could you review this then?

@smengcl
Copy link
Contributor

smengcl commented Jan 9, 2024

smengcl requested a review from GeorgeJahad

@smengcl I'm on vacation and won't have time to look at this till I get back, (in 2 weeks.)

Ah, got it. Enjoy you vacation!

@smengcl , could you review this then?

Yup, on it.

Copy link
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 for the patch. Nice map design here. flushCount works even when it overflows (2^31). I do have a few comments inline.

@szetszwo
Copy link
Contributor Author

szetszwo commented Jan 9, 2024

@smengcl , thanks a lot for reviewing this! Just have pushed a commit addressing your comment.

Copy link
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 for the improvement! Latest revision lgtm.

@adoroszlai adoroszlai merged commit b3a18ae into apache:master Jan 10, 2024
@adoroszlai
Copy link
Contributor

Thanks @szetszwo for the patch, @smengcl for the review.

@szetszwo
Copy link
Contributor Author

@smengcl , thanks for reviewing this!

@adoroszlai , thanks for merging this!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants