Skip to content

Conversation

@jojochuang
Copy link
Contributor

What changes were proposed in this pull request?

HDDS-10564. Make Outputstream writeExecutor daemon threads.

Please describe your PR in detail:
HDDS-10383 added new thread pools in the output stream. They should be made daemon threads so that processes don't wait for them when exiting. (The bug is in the command line which does not close FileSystem object. Nonetheless, it would still be a good idea to make these thread pools daemon)

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-10564

How was this patch tested?

Deployed on a real cluster, with HBase on Ozone, repeated the same command to verify that the command no longer hangs.

Change-Id: Id5a1d332f8ac93991fd52b85ad186cbc6af9630f
@jojochuang jojochuang requested a review from xichen01 March 21, 2024 23:53
@xichen01
Copy link
Contributor

It looks like the threadpool in the RpcClient has never been daemon, so is there a similar problem in previous versions?

@adoroszlai
Copy link
Contributor

so that processes don't wait for them when exiting.

Does that mean ongoing write may be dropped and data lost?

@jojochuang
Copy link
Contributor Author

It looks like the threadpool in the RpcClient has never been daemon, so is there a similar problem in previous versions?

writeExecutor was added by HDDS-10383 and is new.

@jojochuang
Copy link
Contributor Author

jojochuang commented Mar 22, 2024

Does that mean ongoing write may be dropped and data lost?

output stream should call close() which will ensure data in the buffer is flushed.
If client doesn't close the output stream then it doesn't guarantee data will be written, I think.

@adoroszlai adoroszlai merged commit 82c02d3 into apache:master Mar 24, 2024
@adoroszlai
Copy link
Contributor

Thanks @jojochuang for the patch, @xichen01 for the review.

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.

3 participants