-
Notifications
You must be signed in to change notification settings - Fork 1.9k
IGNITE-27473: WALMode consistency check and tests #12614
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
base: master
Are you sure you want to change the base?
Conversation
|
|
||
| DataStorageConfiguration dsCfg = ctx.config().getDataStorageConfiguration(); | ||
|
|
||
| if (dsCfg != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove curly braces for one line statement.
|
|
||
| if (locWalMode != rmtWalMode) { | ||
| throw new IgniteCheckedException("WAL mode validation failed. " + | ||
| " Local node(locNodeAddrs=" + U.addressesAsString(locNode) + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Required indent is 4-space characters, currently it is 8.
| WALMode rmtWalMode = n.attribute(ATTR_WAL_MODE); | ||
|
|
||
| if (locWalMode != rmtWalMode) { | ||
| throw new IgniteCheckedException("WAL mode validation failed. " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest making the error message consistent with existing ones, for example:
throw new IgniteCheckedException("Remote node has WAL mode different from local " +
"[locId8=" + U.id8(locNode.id()) + ", locWalMode=" + locWalMode +
", rmtId8=" + U.id8(n.id()) + ", rmtWalMode=" + rmtWalMode +
", rmtAddrs=" + U.addressesAsString(n) + ", rmtNode=" + U.toShortString(n) + "]");
| /** | ||
| * Tests for WAL mode consistency validation when nodes join cluster. | ||
| */ | ||
| public class GridDiscoveryManagerWalModeConsistencyTest extends GridCommonAbstractTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A test class needs to be added to the suite.
| assertEquals(1, ignite0.cluster().nodes().size()); | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove empty line.
| /** {@inheritDoc} */ | ||
| @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception { | ||
| IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName) | ||
| .setGridLogger(testLog); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can be placed in one line.
| walMode = WALMode.FSYNC; | ||
|
|
||
| LogListener walModeErrorLsnr = LogListener.matches("WAL mode validation failed") | ||
| .andMatches("LOG_ONLY") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Required indent is 4-space characters.
Thank you for submitting the pull request to the Apache Ignite.
In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:
The Contribution Checklist
The description explains WHAT and WHY was made instead of HOW.
The following pattern must be used:
IGNITE-XXXX Change summarywhereXXXX- number of JIRA issue.(see the Maintainers list)
the
green visaattached to the JIRA ticket (see TC.Bot: Check PR)Notes
If you need any help, please email [email protected] or ask anу advice on http://asf.slack.com #ignite channel.