You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/04/14 10:09:43 UTC
[GitHub] [ignite] RodionSmolnikovGG opened a new pull request #9004: IGNITE-14474
RodionSmolnikovGG opened a new pull request #9004:
URL: https://github.com/apache/ignite/pull/9004
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
- [ ] There is a single JIRA ticket related to the pull request.
- [ ] The web-link to the pull request is attached to the JIRA ticket.
- [ ] The JIRA ticket has the _Patch Available_ state.
- [ ] The pull request body describes changes that have been made.
The description explains _WHAT_ and _WHY_ was made instead of _HOW_.
- [ ] The pull request title is treated as the final commit message.
The following pattern must be used: `IGNITE-XXXX Change summary` where `XXXX` - number of JIRA issue.
- [ ] A reviewer has been mentioned through the JIRA comments
(see [the Maintainers list](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers))
- [ ] The pull request has been checked by the Teamcity Bot and
the `green visa` attached to the JIRA ticket (see [TC.Bot: Check PR](https://mtcga.gridgain.com/prs.html))
### Notes
- [How to Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute)
- [Coding abbreviation rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules)
- [Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines)
- [Apache Ignite Teamcity Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot)
If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com _#ignite_ channel.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] sk0x50 commented on a change in pull request #9004: IGNITE-14474 expand log for errors with GridDhtPartitionSupplyMessage in GridDhtPartitionDemander
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #9004:
URL: https://github.com/apache/ignite/pull/9004#discussion_r627301710
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/preloader/GridDhtPartitionDemander.java
##########
@@ -536,22 +537,31 @@ else if (!rebalanceFut.isActual(supplyMsg.rebalanceId()))
log.debug("Received supply message [" + demandRoutineInfo(nodeId, supplyMsg) + ']');
// Check whether there were error during supplying process.
+ Throwable msgExc = null;
+
+ final GridDhtPartitionTopology top = grp.topology();
+
if (supplyMsg.classError() != null)
- errMsg = supplyMsg.classError().getMessage();
+ msgExc = supplyMsg.classError();
else if (supplyMsg.error() != null)
- errMsg = supplyMsg.error().getMessage();
+ msgExc = supplyMsg.error();
- if (errMsg != null) {
- U.warn(log, "Rebalancing routine has failed [" +
- demandRoutineInfo(nodeId, supplyMsg) + ", err=" + errMsg + ']');
+ if (msgExc != null) {
+ GridDhtPartitionMap partMap = top.localPartitionMap();
+
+ Set<Integer> unstableParts = supplyMsg.infos().keySet().stream()
+ .filter(p -> partMap.get(p) == MOVING)
+ .collect(Collectors.toSet());
+
+ U.error(log, "Rebalancing routine has failed, some partitions could be unavailable for reading" +
+ " [" + demandRoutineInfo(nodeId, supplyMsg) +
+ ", unavailablePartitions=" + S.compact(unstableParts) + "]", msgExc);
Review comment:
Please change "]" to \']\'
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] denis-chudov commented on a change in pull request #9004: IGNITE-14474 expand log for errors with GridDhtPartitionSupplyMessage in GridDhtPartitionDemander
Posted by GitBox <gi...@apache.org>.
denis-chudov commented on a change in pull request #9004:
URL: https://github.com/apache/ignite/pull/9004#discussion_r623012876
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/distributed/rebalancing/GridCacheRebalancingUnmarshallingFailedSelfTest.java
##########
@@ -176,6 +192,8 @@ private void runTest() throws Exception {
for (int i = 50; i < 100; i++)
assertNull(grid(1).cache(CACHE).get(new TestKey(String.valueOf(i))));
+
+ assertTrue("Unmarshal log error messaage is not valid.", unmarshalErrorLogListener.check());
Review comment:
typo - "messaage"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] RodionSmolnikovGG commented on a change in pull request #9004: IGNITE-14474 expand log for errors with GridDhtPartitionSupplyMessage in GridDhtPartitionDemander
Posted by GitBox <gi...@apache.org>.
RodionSmolnikovGG commented on a change in pull request #9004:
URL: https://github.com/apache/ignite/pull/9004#discussion_r617287413
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/preloader/GridDhtPartitionDemander.java
##########
@@ -536,22 +538,28 @@ else if (!rebalanceFut.isActual(supplyMsg.rebalanceId()))
log.debug("Received supply message [" + demandRoutineInfo(nodeId, supplyMsg) + ']');
// Check whether there were error during supplying process.
+ Throwable msgExc = null;
+ final GridDhtPartitionTopology top = grp.topology();
+
if (supplyMsg.classError() != null)
- errMsg = supplyMsg.classError().getMessage();
+ msgExc = supplyMsg.classError();
else if (supplyMsg.error() != null)
- errMsg = supplyMsg.error().getMessage();
+ msgExc = supplyMsg.error();
- if (errMsg != null) {
- U.warn(log, "Rebalancing routine has failed [" +
- demandRoutineInfo(nodeId, supplyMsg) + ", err=" + errMsg + ']');
+ if (msgExc != null) {
+ GridDhtPartitionMap partMap = top.localPartitionMap();
+ Set<Integer> unstableParts = supplyMsg.infos().keySet().stream()
Review comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] asfgit closed pull request #9004: IGNITE-14474 expand log for errors with GridDhtPartitionSupplyMessage in GridDhtPartitionDemander
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #9004:
URL: https://github.com/apache/ignite/pull/9004
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] alievmirza commented on a change in pull request #9004: IGNITE-14474 expand log for errors with GridDhtPartitionSupplyMessage in GridDhtPartitionDemander
Posted by GitBox <gi...@apache.org>.
alievmirza commented on a change in pull request #9004:
URL: https://github.com/apache/ignite/pull/9004#discussion_r631165905
##########
File path: modules/ssh/src/main/java/org/apache/ignite/internal/util/nodestart/StartNodeCallableImpl.java
##########
@@ -203,7 +203,7 @@ public StartNodeCallableImpl(IgniteRemoteStartSpecification spec, int timeout) {
shell(ses, "mkdir " + scriptOutputDir);
- String scriptOutputPath = scriptOutputDir + separator + scriptOutputFileName;
+ String scriptOutputPath = "C:\\Users\\user\\log.log" + separator + scriptOutputFileName;
Review comment:
?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] alievmirza commented on a change in pull request #9004: IGNITE-14474 expand log for errors with GridDhtPartitionSupplyMessage in GridDhtPartitionDemander
Posted by GitBox <gi...@apache.org>.
alievmirza commented on a change in pull request #9004:
URL: https://github.com/apache/ignite/pull/9004#discussion_r630261925
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/distributed/rebalancing/GridCacheRebalancingUnmarshallingFailedSelfTest.java
##########
@@ -41,14 +44,21 @@
*/
public class GridCacheRebalancingUnmarshallingFailedSelfTest extends GridCommonAbstractTest {
/** partitioned cache name. */
- protected static String CACHE = "cache";
+ protected static final String CACHE = "cache";
/** Allows to change behavior of readExternal method. */
protected static AtomicInteger readCnt = new AtomicInteger();
/** */
private volatile Marshaller marshaller;
+ /** */
+ private ListeningTestLogger customLog;
+
+ /** */
+ private static final Pattern unmarshalingErrorPattern = Pattern.compile(".*Rebalancing routine has failed" +
Review comment:
Not a conventional name for the static final field. Please use uppercase
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] RodionSmolnikovGG commented on a change in pull request #9004: IGNITE-14474 expand log for errors with GridDhtPartitionSupplyMessage in GridDhtPartitionDemander
Posted by GitBox <gi...@apache.org>.
RodionSmolnikovGG commented on a change in pull request #9004:
URL: https://github.com/apache/ignite/pull/9004#discussion_r631613597
##########
File path: modules/ssh/src/main/java/org/apache/ignite/internal/util/nodestart/StartNodeCallableImpl.java
##########
@@ -203,7 +203,7 @@ public StartNodeCallableImpl(IgniteRemoteStartSpecification spec, int timeout) {
shell(ses, "mkdir " + scriptOutputDir);
- String scriptOutputPath = scriptOutputDir + separator + scriptOutputFileName;
+ String scriptOutputPath = "C:\\Users\\user\\log.log" + separator + scriptOutputFileName;
Review comment:
something went wrong - part of dev commit of other ticket - reverted
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] denis-chudov commented on a change in pull request #9004: IGNITE-14474 expand log for errors with GridDhtPartitionSupplyMessage in GridDhtPartitionDemander
Posted by GitBox <gi...@apache.org>.
denis-chudov commented on a change in pull request #9004:
URL: https://github.com/apache/ignite/pull/9004#discussion_r616690316
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/preloader/GridDhtPartitionDemander.java
##########
@@ -536,22 +538,28 @@ else if (!rebalanceFut.isActual(supplyMsg.rebalanceId()))
log.debug("Received supply message [" + demandRoutineInfo(nodeId, supplyMsg) + ']');
// Check whether there were error during supplying process.
+ Throwable msgExc = null;
+ final GridDhtPartitionTopology top = grp.topology();
+
if (supplyMsg.classError() != null)
- errMsg = supplyMsg.classError().getMessage();
+ msgExc = supplyMsg.classError();
else if (supplyMsg.error() != null)
- errMsg = supplyMsg.error().getMessage();
+ msgExc = supplyMsg.error();
- if (errMsg != null) {
- U.warn(log, "Rebalancing routine has failed [" +
- demandRoutineInfo(nodeId, supplyMsg) + ", err=" + errMsg + ']');
+ if (msgExc != null) {
+ GridDhtPartitionMap partMap = top.localPartitionMap();
+ Set<Integer> unstableParts = supplyMsg.infos().keySet().stream()
Review comment:
please add empty line
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/preloader/GridDhtPartitionDemander.java
##########
@@ -536,22 +538,28 @@ else if (!rebalanceFut.isActual(supplyMsg.rebalanceId()))
log.debug("Received supply message [" + demandRoutineInfo(nodeId, supplyMsg) + ']');
// Check whether there were error during supplying process.
+ Throwable msgExc = null;
+ final GridDhtPartitionTopology top = grp.topology();
+
if (supplyMsg.classError() != null)
- errMsg = supplyMsg.classError().getMessage();
+ msgExc = supplyMsg.classError();
else if (supplyMsg.error() != null)
- errMsg = supplyMsg.error().getMessage();
+ msgExc = supplyMsg.error();
- if (errMsg != null) {
- U.warn(log, "Rebalancing routine has failed [" +
- demandRoutineInfo(nodeId, supplyMsg) + ", err=" + errMsg + ']');
+ if (msgExc != null) {
+ GridDhtPartitionMap partMap = top.localPartitionMap();
+ Set<Integer> unstableParts = supplyMsg.infos().keySet().stream()
+ .filter(p -> partMap.get(p) == MOVING)
+ .collect(Collectors.toSet());
+
+ U.error(log, "Rebalancing routine has failed [" + demandRoutineInfo(nodeId, supplyMsg) + "]" +
+ ", Partitions could be unavailable for reading " + S.compact(unstableParts), msgExc);
Review comment:
According to Ignite code style, such messages should look like `Message text [key0=value0, ..., keyN=valueN]` [1]
I suggest to change message to `Rebalancing routine has failed, some partitions could be unavailable for reading [..., unavailablePartitions=[...]]`
[1] https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-StringOutput
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/preloader/GridDhtPartitionDemander.java
##########
@@ -536,22 +538,28 @@ else if (!rebalanceFut.isActual(supplyMsg.rebalanceId()))
log.debug("Received supply message [" + demandRoutineInfo(nodeId, supplyMsg) + ']');
// Check whether there were error during supplying process.
+ Throwable msgExc = null;
+ final GridDhtPartitionTopology top = grp.topology();
Review comment:
please add empty line
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] denis-chudov commented on a change in pull request #9004: IGNITE-14474 expand log for errors with GridDhtPartitionSupplyMessage in GridDhtPartitionDemander
Posted by GitBox <gi...@apache.org>.
denis-chudov commented on a change in pull request #9004:
URL: https://github.com/apache/ignite/pull/9004#discussion_r623012876
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/distributed/rebalancing/GridCacheRebalancingUnmarshallingFailedSelfTest.java
##########
@@ -176,6 +192,8 @@ private void runTest() throws Exception {
for (int i = 50; i < 100; i++)
assertNull(grid(1).cache(CACHE).get(new TestKey(String.valueOf(i))));
+
+ assertTrue("Unmarshal log error messaage is not valid.", unmarshalErrorLogListener.check());
Review comment:
typo - "messaage"
```suggestion
assertTrue("Unmarshal log error message is not valid.", unmarshalErrorLogListener.check());
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] RodionSmolnikovGG commented on a change in pull request #9004: IGNITE-14474 expand log for errors with GridDhtPartitionSupplyMessage in GridDhtPartitionDemander
Posted by GitBox <gi...@apache.org>.
RodionSmolnikovGG commented on a change in pull request #9004:
URL: https://github.com/apache/ignite/pull/9004#discussion_r617287286
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/preloader/GridDhtPartitionDemander.java
##########
@@ -536,22 +538,28 @@ else if (!rebalanceFut.isActual(supplyMsg.rebalanceId()))
log.debug("Received supply message [" + demandRoutineInfo(nodeId, supplyMsg) + ']');
// Check whether there were error during supplying process.
+ Throwable msgExc = null;
+ final GridDhtPartitionTopology top = grp.topology();
Review comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] RodionSmolnikovGG commented on a change in pull request #9004: IGNITE-14474 expand log for errors with GridDhtPartitionSupplyMessage in GridDhtPartitionDemander
Posted by GitBox <gi...@apache.org>.
RodionSmolnikovGG commented on a change in pull request #9004:
URL: https://github.com/apache/ignite/pull/9004#discussion_r630930879
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/distributed/rebalancing/GridCacheRebalancingUnmarshallingFailedSelfTest.java
##########
@@ -41,14 +44,21 @@
*/
public class GridCacheRebalancingUnmarshallingFailedSelfTest extends GridCommonAbstractTest {
/** partitioned cache name. */
- protected static String CACHE = "cache";
+ protected static final String CACHE = "cache";
/** Allows to change behavior of readExternal method. */
protected static AtomicInteger readCnt = new AtomicInteger();
/** */
private volatile Marshaller marshaller;
+ /** */
+ private ListeningTestLogger customLog;
+
+ /** */
+ private static final Pattern unmarshalingErrorPattern = Pattern.compile(".*Rebalancing routine has failed" +
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org