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/10/29 07:36:11 UTC
[GitHub] [ignite-3] alievmirza opened a new pull request #418: IGNITE-15581 add proper handling of errors in user's state machines
alievmirza opened a new pull request #418:
URL: https://github.com/apache/ignite-3/pull/418
https://issues.apache.org/jira/browse/IGNITE-15581
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] vldpyatkov commented on a change in pull request #418: IGNITE-15581 add proper handling of errors in user's state machines
Posted by GitBox <gi...@apache.org>.
vldpyatkov commented on a change in pull request #418:
URL: https://github.com/apache/ignite-3/pull/418#discussion_r740989503
##########
File path: modules/raft/src/main/java/org/apache/ignite/raft/jraft/rpc/impl/RaftGroupServiceImpl.java
##########
@@ -546,6 +547,23 @@ else if (resp0.errorCode() == RaftError.EPERM.getNumber() ||
fut.completeExceptionally(
new RaftException(RaftError.forNumber(resp0.errorCode()), resp0.errorMsg()));
}
+ else if (resp instanceof RpcRequests.SMErrorResponse) {
+ SMThrowable th = ((RpcRequests.SMErrorResponse)resp).error();
+ if (th instanceof SMCompactedThrowable) {
+ try {
+ Throwable restoredTh = (Throwable)Class.forName(((SMCompactedThrowable)th).throwableClassName())
+ .getConstructor(String.class)
+ .newInstance(((SMCompactedThrowable)th).throwableMessage());
+
+ fut.completeExceptionally(restoredTh);
+ }
+ catch (Exception e) {
Review comment:
Can you write a warning here?
"The exception does not have a constructor with a string message, IgniteException will be use."
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] alievmirza commented on a change in pull request #418: IGNITE-15581 add proper handling of errors in user's state machines
Posted by GitBox <gi...@apache.org>.
alievmirza commented on a change in pull request #418:
URL: https://github.com/apache/ignite-3/pull/418#discussion_r740778773
##########
File path: modules/metastorage-client/src/integrationTest/java/org/apache/ignite/internal/metastorage/client/ITMetaStorageServiceTest.java
##########
@@ -602,6 +601,25 @@ public void testRangeNext() {
assertEquals(EXPECTED_RESULT_ENTRY, cursor.iterator().next());
}
+ /**
+ * Tests {@link MetaStorageService#range(ByteArray, ByteArray, long)}'s cursor exceptional case.
+ */
+ @Test
+ public void testRangeNextNoSuchElementException() {
+ when(mockStorage.range(EXPECTED_RESULT_ENTRY.key().bytes(), null)).thenAnswer(invocation -> {
Review comment:
this is a valid case when the cursor on the server side is not found
##########
File path: modules/metastorage-client/src/integrationTest/java/org/apache/ignite/internal/metastorage/client/ITMetaStorageServiceTest.java
##########
@@ -602,6 +601,25 @@ public void testRangeNext() {
assertEquals(EXPECTED_RESULT_ENTRY, cursor.iterator().next());
}
+ /**
+ * Tests {@link MetaStorageService#range(ByteArray, ByteArray, long)}'s cursor exceptional case.
+ */
+ @Test
+ public void testRangeNextNoSuchElementException() {
+ when(mockStorage.range(EXPECTED_RESULT_ENTRY.key().bytes(), null)).thenAnswer(invocation -> {
Review comment:
this is a valid case when the cursor on the server side is not found, for example
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] alievmirza commented on a change in pull request #418: IGNITE-15581 add proper handling of errors in user's state machines
Posted by GitBox <gi...@apache.org>.
alievmirza commented on a change in pull request #418:
URL: https://github.com/apache/ignite-3/pull/418#discussion_r740778773
##########
File path: modules/metastorage-client/src/integrationTest/java/org/apache/ignite/internal/metastorage/client/ITMetaStorageServiceTest.java
##########
@@ -602,6 +601,25 @@ public void testRangeNext() {
assertEquals(EXPECTED_RESULT_ENTRY, cursor.iterator().next());
}
+ /**
+ * Tests {@link MetaStorageService#range(ByteArray, ByteArray, long)}'s cursor exceptional case.
+ */
+ @Test
+ public void testRangeNextNoSuchElementException() {
+ when(mockStorage.range(EXPECTED_RESULT_ENTRY.key().bytes(), null)).thenAnswer(invocation -> {
Review comment:
this is a valid case when the cursor on the server side is not found
##########
File path: modules/metastorage-client/src/integrationTest/java/org/apache/ignite/internal/metastorage/client/ITMetaStorageServiceTest.java
##########
@@ -602,6 +601,25 @@ public void testRangeNext() {
assertEquals(EXPECTED_RESULT_ENTRY, cursor.iterator().next());
}
+ /**
+ * Tests {@link MetaStorageService#range(ByteArray, ByteArray, long)}'s cursor exceptional case.
+ */
+ @Test
+ public void testRangeNextNoSuchElementException() {
+ when(mockStorage.range(EXPECTED_RESULT_ENTRY.key().bytes(), null)).thenAnswer(invocation -> {
Review comment:
this is a valid case when the cursor on the server side is not found, for example
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] vldpyatkov commented on a change in pull request #418: IGNITE-15581 add proper handling of errors in user's state machines
Posted by GitBox <gi...@apache.org>.
vldpyatkov commented on a change in pull request #418:
URL: https://github.com/apache/ignite-3/pull/418#discussion_r740989503
##########
File path: modules/raft/src/main/java/org/apache/ignite/raft/jraft/rpc/impl/RaftGroupServiceImpl.java
##########
@@ -546,6 +547,23 @@ else if (resp0.errorCode() == RaftError.EPERM.getNumber() ||
fut.completeExceptionally(
new RaftException(RaftError.forNumber(resp0.errorCode()), resp0.errorMsg()));
}
+ else if (resp instanceof RpcRequests.SMErrorResponse) {
+ SMThrowable th = ((RpcRequests.SMErrorResponse)resp).error();
+ if (th instanceof SMCompactedThrowable) {
+ try {
+ Throwable restoredTh = (Throwable)Class.forName(((SMCompactedThrowable)th).throwableClassName())
+ .getConstructor(String.class)
+ .newInstance(((SMCompactedThrowable)th).throwableMessage());
+
+ fut.completeExceptionally(restoredTh);
+ }
+ catch (Exception e) {
Review comment:
Can you write a warning here?
"The exception does not have a constructor with a string message, IgniteException will be use."
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] vldpyatkov commented on a change in pull request #418: IGNITE-15581 add proper handling of errors in user's state machines
Posted by GitBox <gi...@apache.org>.
vldpyatkov commented on a change in pull request #418:
URL: https://github.com/apache/ignite-3/pull/418#discussion_r740989503
##########
File path: modules/raft/src/main/java/org/apache/ignite/raft/jraft/rpc/impl/RaftGroupServiceImpl.java
##########
@@ -546,6 +547,23 @@ else if (resp0.errorCode() == RaftError.EPERM.getNumber() ||
fut.completeExceptionally(
new RaftException(RaftError.forNumber(resp0.errorCode()), resp0.errorMsg()));
}
+ else if (resp instanceof RpcRequests.SMErrorResponse) {
+ SMThrowable th = ((RpcRequests.SMErrorResponse)resp).error();
+ if (th instanceof SMCompactedThrowable) {
+ try {
+ Throwable restoredTh = (Throwable)Class.forName(((SMCompactedThrowable)th).throwableClassName())
+ .getConstructor(String.class)
+ .newInstance(((SMCompactedThrowable)th).throwableMessage());
+
+ fut.completeExceptionally(restoredTh);
+ }
+ catch (Exception e) {
Review comment:
Can you write a warning here?
"The exception does not have a constructor with a string message, IgniteException will be use."
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] asfgit closed pull request #418: IGNITE-15581 add proper handling of errors in user's state machines
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #418:
URL: https://github.com/apache/ignite-3/pull/418
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] vldpyatkov commented on a change in pull request #418: IGNITE-15581 add proper handling of errors in user's state machines
Posted by GitBox <gi...@apache.org>.
vldpyatkov commented on a change in pull request #418:
URL: https://github.com/apache/ignite-3/pull/418#discussion_r740014482
##########
File path: modules/raft-client/src/main/java/org/apache/ignite/raft/client/service/CommandClosure.java
##########
@@ -37,5 +37,14 @@
* Must be called after a command has been processed normally.
* @param res Execution result.
*/
- void result(@Nullable Serializable res);
+ void success(@Nullable Serializable res);
+
+ /**
+ * Must be called after a command has failed.
+ * @param th Throwable
+ * @param compacted True if {@code th} must be compacted before sending over network. Compaction means
+ * that throwable will be changed to a more compact structure, containing only message. Note, that
+ * in case of compaction, corresponding throwable must be in classpath on a client side.
+ */
+ void failure(Throwable th, boolean compacted);
Review comment:
Parameter compacted seems doubt to me.
Because it is a technical field it depends of some internal configuration (need stack on client of not, may be due to debug level). I do not wont to see this argument in the API.
##########
File path: modules/metastorage-client/src/integrationTest/java/org/apache/ignite/internal/metastorage/client/ITMetaStorageServiceTest.java
##########
@@ -602,6 +601,25 @@ public void testRangeNext() {
assertEquals(EXPECTED_RESULT_ENTRY, cursor.iterator().next());
}
+ /**
+ * Tests {@link MetaStorageService#range(ByteArray, ByteArray, long)}'s cursor exceptional case.
+ */
+ @Test
+ public void testRangeNextNoSuchElementException() {
+ when(mockStorage.range(EXPECTED_RESULT_ENTRY.key().bytes(), null)).thenAnswer(invocation -> {
Review comment:
Why can't get NoSuchElementException honestly? (Iterate until a cursor finished)
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] alievmirza commented on a change in pull request #418: IGNITE-15581 add proper handling of errors in user's state machines
Posted by GitBox <gi...@apache.org>.
alievmirza commented on a change in pull request #418:
URL: https://github.com/apache/ignite-3/pull/418#discussion_r740778773
##########
File path: modules/metastorage-client/src/integrationTest/java/org/apache/ignite/internal/metastorage/client/ITMetaStorageServiceTest.java
##########
@@ -602,6 +601,25 @@ public void testRangeNext() {
assertEquals(EXPECTED_RESULT_ENTRY, cursor.iterator().next());
}
+ /**
+ * Tests {@link MetaStorageService#range(ByteArray, ByteArray, long)}'s cursor exceptional case.
+ */
+ @Test
+ public void testRangeNextNoSuchElementException() {
+ when(mockStorage.range(EXPECTED_RESULT_ENTRY.key().bytes(), null)).thenAnswer(invocation -> {
Review comment:
this is a valid case when the cursor on the server side is not found
##########
File path: modules/metastorage-client/src/integrationTest/java/org/apache/ignite/internal/metastorage/client/ITMetaStorageServiceTest.java
##########
@@ -602,6 +601,25 @@ public void testRangeNext() {
assertEquals(EXPECTED_RESULT_ENTRY, cursor.iterator().next());
}
+ /**
+ * Tests {@link MetaStorageService#range(ByteArray, ByteArray, long)}'s cursor exceptional case.
+ */
+ @Test
+ public void testRangeNextNoSuchElementException() {
+ when(mockStorage.range(EXPECTED_RESULT_ENTRY.key().bytes(), null)).thenAnswer(invocation -> {
Review comment:
this is a valid case when the cursor on the server side is not found, for example
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org