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/12/02 15:18:47 UTC
[GitHub] [ignite-3] vldpyatkov opened a new pull request #488: IGNITE-16033 Wrong completion of an alter table operation for a slowi…
vldpyatkov opened a new pull request #488:
URL: https://github.com/apache/ignite-3/pull/488
…ng node
--
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] sanpwc commented on a change in pull request #488: IGNITE-16033 Wrong completion of an alter table operation for a slowi…
Posted by GitBox <gi...@apache.org>.
sanpwc commented on a change in pull request #488:
URL: https://github.com/apache/ignite-3/pull/488#discussion_r761962653
##########
File path: modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java
##########
@@ -937,25 +874,49 @@ public void remove(@NotNull Throwable e) {
schemaCh.changeSchema(SchemaSerializerImpl.INSTANCE.serialize(descriptor));
}));
}
- )).exceptionally(t -> {
- LOG.error(LoggerMessageHelper.format("Table wasn't altered [name={}]", name), t);
+ )).whenComplete((res, t) -> {
+ if (t != null) {
+ Throwable ex = getRootCause(t);
- Throwable cause = t.getCause();
+ assert ex != null;
- if (cause instanceof ConfigurationChangeException) {
- cause = cause.getCause();
- }
+ LOG.error(LoggerMessageHelper.format("Table wasn't altered [name={}]", name), ex);
- removeListener(TableEvent.ALTER, clo, new IgniteInternalCheckedException(cause));
-
- return null;
+ tblFut.completeExceptionally(ex);
+ } else {
+ tblFut.complete(res);
+ }
});
}
});
return tblFut;
}
+ /**
+ * Gets a cause exception for a client.
+ *
+ * @param t Exception wrapper.
+ * @return Root exception if the exception is wrapped on.
+ */
+ private Throwable getRootCause(Throwable t) {
+ Throwable ex;
+
+ if (t instanceof CompletionException) {
+ if (t.getCause() instanceof ConfigurationChangeException) {
+ //TODO: IGNITE-16051 Only public exception for configuration validation should return here.
Review comment:
Seems to be no longer valid.
--
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] sanpwc commented on a change in pull request #488: IGNITE-16033 Wrong completion of an alter table operation for a slowi…
Posted by GitBox <gi...@apache.org>.
sanpwc commented on a change in pull request #488:
URL: https://github.com/apache/ignite-3/pull/488#discussion_r761965757
##########
File path: modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java
##########
@@ -937,25 +874,49 @@ public void remove(@NotNull Throwable e) {
schemaCh.changeSchema(SchemaSerializerImpl.INSTANCE.serialize(descriptor));
}));
}
- )).exceptionally(t -> {
- LOG.error(LoggerMessageHelper.format("Table wasn't altered [name={}]", name), t);
+ )).whenComplete((res, t) -> {
+ if (t != null) {
+ Throwable ex = getRootCause(t);
- Throwable cause = t.getCause();
+ assert ex != null;
- if (cause instanceof ConfigurationChangeException) {
- cause = cause.getCause();
- }
+ LOG.error(LoggerMessageHelper.format("Table wasn't altered [name={}]", name), ex);
- removeListener(TableEvent.ALTER, clo, new IgniteInternalCheckedException(cause));
-
- return null;
+ tblFut.completeExceptionally(ex);
+ } else {
+ tblFut.complete(res);
+ }
});
}
});
return tblFut;
}
+ /**
+ * Gets a cause exception for a client.
+ *
+ * @param t Exception wrapper.
+ * @return Root exception if the exception is wrapped on.
+ */
+ private Throwable getRootCause(Throwable t) {
+ Throwable ex;
+
+ if (t instanceof CompletionException) {
+ if (t.getCause() instanceof ConfigurationChangeException) {
+ //TODO: IGNITE-16051 Only public exception for configuration validation should return here.
+ return t.getCause().getCause();
+ } else {
+ ex = t.getCause();
+ }
+
+ } else {
+ ex = t;
+ }
+
+ return ex instanceof IgniteException ? (IgniteException) ex : new IgniteException(ex);
Review comment:
What if it's common java exception? Should we wrap it with IgniteException one?
What if someone will add public IgniteCheckedExcpetion (or maybe it's even exists already), should we wrap it with IgniteException?
--
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 #488: IGNITE-16033 Wrong completion of an alter table operation for a slowi…
Posted by GitBox <gi...@apache.org>.
vldpyatkov commented on a change in pull request #488:
URL: https://github.com/apache/ignite-3/pull/488#discussion_r761978486
##########
File path: modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java
##########
@@ -937,25 +874,49 @@ public void remove(@NotNull Throwable e) {
schemaCh.changeSchema(SchemaSerializerImpl.INSTANCE.serialize(descriptor));
}));
}
- )).exceptionally(t -> {
- LOG.error(LoggerMessageHelper.format("Table wasn't altered [name={}]", name), t);
+ )).whenComplete((res, t) -> {
+ if (t != null) {
+ Throwable ex = getRootCause(t);
- Throwable cause = t.getCause();
+ assert ex != null;
- if (cause instanceof ConfigurationChangeException) {
- cause = cause.getCause();
- }
+ LOG.error(LoggerMessageHelper.format("Table wasn't altered [name={}]", name), ex);
- removeListener(TableEvent.ALTER, clo, new IgniteInternalCheckedException(cause));
-
- return null;
+ tblFut.completeExceptionally(ex);
+ } else {
+ tblFut.complete(res);
+ }
});
}
});
return tblFut;
}
+ /**
+ * Gets a cause exception for a client.
+ *
+ * @param t Exception wrapper.
+ * @return Root exception if the exception is wrapped on.
+ */
+ private Throwable getRootCause(Throwable t) {
+ Throwable ex;
+
+ if (t instanceof CompletionException) {
+ if (t.getCause() instanceof ConfigurationChangeException) {
+ //TODO: IGNITE-16051 Only public exception for configuration validation should return here.
Review comment:
I can reformulate it, but still sure only public exception can be a cause.
--
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 #488: IGNITE-16033 Wrong completion of an alter table operation for a slowi…
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #488:
URL: https://github.com/apache/ignite-3/pull/488
--
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