Skip to content
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.

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

HDDS-3477. Disable partial chunk write during flush() call in ozone client by default. #957

Merged
merged 8 commits into from
Jun 3, 2020

Conversation

captainzmc
Copy link
Member

What changes were proposed in this pull request?

Currently, Ozone client flushes the partial chunks as well during flush() call by default.

#716 proposes to add a configuration to disallow partial chunk flush during flush() call. This Jira aims to enable the config on by default to mimic the default hdfs flush() behaviour and fix any failing unit tests associated with the change.

What is the link to the Apache JIRA

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

How was this patch tested?

The affected UT has been modified.

@captainzmc
Copy link
Member Author

Hi @bshashikant, Could you please help to review this PR?

@captainzmc captainzmc closed this May 22, 2020
@captainzmc captainzmc reopened this May 22, 2020
Copy link
Contributor

@bshashikant bshashikant left a comment

Choose a reason for hiding this comment

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

Thanks @captainzmc for working on this. The changes look ok. However, i would prefer to not fix the actual unit tests but set the flush config to false in these unit tests. We can rewrite the same test with the flush configuration set to true . This way, unit tests will have coverage for different behaviours of flush with/without the configuration turned on. What do you think?

@captainzmc
Copy link
Member Author

Thanks @captainzmc for working on this. The changes look ok. However, i would prefer to not fix the actual unit tests but set the flush config to false in these unit tests. We can rewrite the same test with the flush configuration set to true . This way, unit tests will have coverage for different behaviours of flush with/without the configuration turned on. What do you think?

Agree, that will make the test more complete.
I will set flush config to false in the original UT. And rewrite the same test with the flush configuration set to true .

@captainzmc captainzmc force-pushed the HDDS-3477 branch 2 times, most recently from 734fc7d to c87aba5 Compare June 2, 2020 13:26
@captainzmc captainzmc closed this Jun 2, 2020
@captainzmc captainzmc reopened this Jun 2, 2020
@captainzmc
Copy link
Member Author

Had set flush config to false in the original UT. And rewrite the same test with the flush configuration set to true. cc @bshashikant

@bshashikant bshashikant merged commit e4f23ee into apache:master Jun 3, 2020
@bshashikant
Copy link
Contributor

Thanks @captainzmc for the contribution. I have committed this to master branch.

isahekmat pushed a commit to isahekmat/hadoop-ozone that referenced this pull request Jun 4, 2020
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.

2 participants