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-3380. MiniOzoneHAClusterImpl#initOMRatisConf will reset the configs and causes for test failures #817

Merged
merged 9 commits into from Apr 21, 2020

Conversation

umamaheswararao
Copy link
Contributor

…igs and causes for test failures

What changes were proposed in this pull request?

Moved the configs settings only to required test class instead of silently resetting test modified configs. Also took liberty to change assertion from NOT_A_FILE to DIR_NOT_FOUND, which seems correct code.

What is the link to the Apache JIRA

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

How was this patch tested?

Ran the TestOzoneManagerHA tests. SnapShot test is passing after fixing resetting issue.

@@ -470,6 +468,7 @@ public void testCreateFile(OzoneBucket ozoneBucket, String keyName,
Assert.assertEquals(data, new String(fileContent));
}

@Ignore("This test failing randomly and triggering HDDS-3465. ` `4qa wq")
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: Remove 4qa wq"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Damn, somehow I pressed some unwanted chars before commit. I will change it.

@@ -93,8 +93,7 @@
.OZONE_CLIENT_FAILOVER_SLEEP_BASE_MILLIS_DEFAULT;
import static org.apache.hadoop.ozone.OzoneConfigKeys
.OZONE_OPEN_KEY_EXPIRE_THRESHOLD_SECONDS;
import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.FILE_ALREADY_EXISTS;
import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.NOT_A_FILE;
import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.*;
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor NIT: Do not use *

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that was done by Intellij auto.

@@ -383,6 +383,7 @@ public MiniOzoneChaosCluster build() throws IOException {

DefaultMetricsSystem.setMiniClusterMode(true);
initializeConfiguration();

Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: Only space change

conf.setTimeDuration(
OMConfigKeys.OZONE_OM_LEADER_ELECTION_MINIMUM_TIMEOUT_DURATION_KEY,
LEADER_ELECTION_TIMEOUT, TimeUnit.MILLISECONDS);
conf.set(ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY, "127." +
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: Can we move this to single line

Copy link
Contributor

@bharatviswa504 bharatviswa504 left a comment

Choose a reason for hiding this comment

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

+1 LGTM.
Few minor comments.

RATIS_LEADER_ELECTION_TIMEOUT, TimeUnit.MILLISECONDS);
// If test change the following config values we will respect,
// otherwise we will set lower timeout values.
long defaultDuration = OMConfigKeys
Copy link
Contributor

Choose a reason for hiding this comment

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

Formatting is not proper here for these code lines, few of them can fit in a single line.

Copy link
Contributor

@bharatviswa504 bharatviswa504 left a comment

Choose a reason for hiding this comment

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

+1 LGTM.

@umamaheswararao umamaheswararao merged commit ce94889 into apache:master Apr 21, 2020
@umamaheswararao umamaheswararao changed the title HDDS-3380. MiniOzoneHAClusterImpl#initOMRatisConf will reset the conf… HDDS-3380. MiniOzoneHAClusterImpl#initOMRatisConf will reset the configs and causes for test failures Apr 21, 2020
@umamaheswararao umamaheswararao deleted the HATests branch April 30, 2020 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants