You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by sk...@apache.org on 2021/12/06 07:05:41 UTC
[ignite-3] branch main updated: IGNITE-16033 Fixed wrong completion stage related to an alter table operation. Fixes #488
This is an automated email from the ASF dual-hosted git repository.
sk0x50 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new 1141cad IGNITE-16033 Fixed wrong completion stage related to an alter table operation. Fixes #488
1141cad is described below
commit 1141cad4f1e43875d114e1360299781a57d34f61
Author: Vladislav Pyatkov <vl...@gmail.com>
AuthorDate: Mon Dec 6 10:05:11 2021 +0300
IGNITE-16033 Fixed wrong completion stage related to an alter table operation. Fixes #488
Signed-off-by: Slava Koptilin <sl...@gmail.com>
---
.../runner/app/AbstractSchemaChangeTest.java | 3 +-
.../runner/app/ItDynamicTableCreationTest.java | 3 +-
.../internal/runner/app/ItTablesApiTest.java | 3 -
.../internal/table/distributed/TableManager.java | 172 +++++++--------------
4 files changed, 56 insertions(+), 125 deletions(-)
diff --git a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/AbstractSchemaChangeTest.java b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/AbstractSchemaChangeTest.java
index b7c9846..b8edfb2 100644
--- a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/AbstractSchemaChangeTest.java
+++ b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/AbstractSchemaChangeTest.java
@@ -32,7 +32,6 @@ import java.util.function.Supplier;
import org.apache.ignite.Ignite;
import org.apache.ignite.IgnitionManager;
import org.apache.ignite.configuration.schemas.table.ColumnChange;
-import org.apache.ignite.configuration.validation.ConfigurationValidationException;
import org.apache.ignite.internal.ItUtils;
import org.apache.ignite.internal.testframework.WorkDirectory;
import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
@@ -270,7 +269,7 @@ abstract class AbstractSchemaChangeTest {
* @param colChanger Column configuration changer.
*/
private static void assertColumnChangeFailed(List<Ignite> grid, String colName, Consumer<ColumnChange> colChanger) {
- assertThrows(ConfigurationValidationException.class, () ->
+ assertThrows(IgniteException.class, () ->
grid.get(0).tables().alterTable(TABLE,
tblChanger -> tblChanger.changeColumns(listChanger -> listChanger.update(colName, colChanger))
)
diff --git a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItDynamicTableCreationTest.java b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItDynamicTableCreationTest.java
index c2c287f..83281c9 100644
--- a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItDynamicTableCreationTest.java
+++ b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItDynamicTableCreationTest.java
@@ -39,6 +39,7 @@ import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConver
import org.apache.ignite.internal.testframework.WorkDirectory;
import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.lang.IgniteException;
import org.apache.ignite.schema.SchemaBuilders;
import org.apache.ignite.schema.definition.ColumnType;
import org.apache.ignite.schema.definition.TableDefinition;
@@ -369,7 +370,7 @@ class ItDynamicTableCreationTest {
* @param colChanger Column configuration changer.
*/
private void assertTableCreationFailed(List<Ignite> grid, Consumer<ColumnChange> colChanger) {
- Assertions.assertThrows(ConfigurationValidationException.class, () -> {
+ Assertions.assertThrows(IgniteException.class, () -> {
try {
grid.get(0).tables().createTable(
"PUBLIC.tbl1",
diff --git a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItTablesApiTest.java b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItTablesApiTest.java
index 75f321e..8b7fde2 100644
--- a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItTablesApiTest.java
+++ b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItTablesApiTest.java
@@ -54,7 +54,6 @@ import org.apache.ignite.schema.definition.index.IndexDefinition;
import org.apache.ignite.table.Table;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInfo;
@@ -235,7 +234,6 @@ public class ItTablesApiTest extends IgniteAbstractTest {
* @throws Exception If failed.
*/
@Test
- @Disabled("IGNITE-16033")
public void testAddIndexFromLaggedNode() throws Exception {
clusterNodes.forEach(ign -> assertNull(ign.tables().table(TABLE_NAME)));
@@ -304,7 +302,6 @@ public class ItTablesApiTest extends IgniteAbstractTest {
* @throws Exception If failed.
*/
@Test
- @Disabled("IGNITE-16033")
public void testAddColumnFromLaggedNode() throws Exception {
clusterNodes.forEach(ign -> assertNull(ign.tables().table(TABLE_NAME)));
diff --git a/modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java b/modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java
index bb8f45a..a8d02c3 100644
--- a/modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java
+++ b/modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java
@@ -83,7 +83,6 @@ import org.apache.ignite.internal.tx.TxManager;
import org.apache.ignite.internal.util.ByteUtils;
import org.apache.ignite.internal.util.IgniteSpinBusyLock;
import org.apache.ignite.lang.IgniteException;
-import org.apache.ignite.lang.IgniteInternalCheckedException;
import org.apache.ignite.lang.IgniteInternalException;
import org.apache.ignite.lang.IgniteLogger;
import org.apache.ignite.lang.IgniteUuid;
@@ -720,32 +719,6 @@ public class TableManager extends Producer<TableEvent, TableEventParameters> imp
IgniteUuid tblId = TABLE_ID_GENERATOR.randomUuid();
- EventListener<TableEventParameters> clo = new EventListener<>() {
- @Override
- public boolean notify(@NotNull TableEventParameters parameters, @Nullable Throwable e) {
- IgniteUuid notificationTblId = parameters.tableId();
-
- if (!tblId.equals(notificationTblId)) {
- return false;
- }
-
- if (e == null) {
- tblFut.complete(parameters.table());
- } else {
- tblFut.completeExceptionally(e);
- }
-
- return true;
- }
-
- @Override
- public void remove(@NotNull Throwable e) {
- tblFut.completeExceptionally(e);
- }
- };
-
- listen(TableEvent.CREATE, clo);
-
tablesCfg.tables().change(change -> {
if (change.get(name) != null) {
throw new TableAlreadyExistsException(name);
@@ -784,36 +757,24 @@ public class TableManager extends Producer<TableEvent, TableEventParameters> imp
));
}
);
- }).exceptionally(t -> {
- Throwable ex;
-
- if (t instanceof CompletionException) {
- if (t.getCause() instanceof ConfigurationChangeException) {
- ex = t.getCause().getCause();
- } else {
- ex = t.getCause();
- }
+ }).whenComplete((res, t) -> {
+ if (t != null) {
+ Throwable ex = getRootCause(t);
- assert ex != null;
- } else {
- ex = t;
- }
-
- if (ex instanceof TableAlreadyExistsException) {
- tableAsync(name, false).thenAccept(table -> {
- if (!exceptionWhenExist) {
- tblFut.complete(table);
+ if (ex instanceof TableAlreadyExistsException) {
+ if (exceptionWhenExist) {
+ tblFut.completeExceptionally(ex);
+ } else {
+ tblFut.complete(tables.get(name));
}
+ } else {
+ LOG.error(LoggerMessageHelper.format("Table wasn't created [name={}]", name), ex);
- removeListener(TableEvent.CREATE, clo, new IgniteInternalCheckedException(ex));
- });
+ tblFut.completeExceptionally(ex);
+ }
} else {
- LOG.error(LoggerMessageHelper.format("Table wasn't created [name={}]", name), t);
-
- removeListener(TableEvent.CREATE, clo, new IgniteInternalCheckedException(ex));
+ tblFut.complete(tables.get(name));
}
-
- return null;
});
return tblFut;
@@ -875,32 +836,6 @@ public class TableManager extends Producer<TableEvent, TableEventParameters> imp
} else {
IgniteUuid tblId = ((TableImpl) tbl).tableId();
- EventListener<TableEventParameters> clo = new EventListener<>() {
- @Override
- public boolean notify(@NotNull TableEventParameters parameters, @Nullable Throwable e) {
- IgniteUuid notificationTblId = parameters.tableId();
-
- if (!tblId.equals(notificationTblId)) {
- return false;
- }
-
- if (e == null) {
- tblFut.complete(null);
- } else {
- tblFut.completeExceptionally(e);
- }
-
- return true;
- }
-
- @Override
- public void remove(@NotNull Throwable e) {
- tblFut.completeExceptionally(e);
- }
- };
-
- listen(TableEvent.ALTER, clo);
-
tablesCfg.tables().change(ch -> ch.createOrUpdate(name, tblCh -> {
tableChange.accept(tblCh);
@@ -937,18 +872,16 @@ public class TableManager extends Producer<TableEvent, TableEventParameters> imp
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();
+ LOG.error(LoggerMessageHelper.format("Table wasn't altered [name={}]", name), ex);
- if (cause instanceof ConfigurationChangeException) {
- cause = cause.getCause();
+ tblFut.completeExceptionally(ex);
+ } else {
+ tblFut.complete(res);
}
-
- removeListener(TableEvent.ALTER, clo, new IgniteInternalCheckedException(cause));
-
- return null;
});
}
});
@@ -956,6 +889,30 @@ public class TableManager extends Producer<TableEvent, TableEventParameters> imp
return tblFut;
}
+ /**
+ * Gets a cause exception for a client.
+ *
+ * @param t Exception wrapper.
+ * @return A root exception which will be acceptable to throw for public API.
+ */
+ //TODO: IGNITE-16051 Implement exception converter for public API.
+ private @NotNull IgniteException getRootCause(Throwable t) {
+ Throwable ex;
+
+ if (t instanceof CompletionException) {
+ if (t.getCause() instanceof ConfigurationChangeException) {
+ ex = t.getCause().getCause();
+ } else {
+ ex = t.getCause();
+ }
+
+ } else {
+ ex = t;
+ }
+
+ return ex instanceof IgniteException ? (IgniteException) ex : new IgniteException(ex);
+ }
+
/** {@inheritDoc} */
@Override
public void dropTable(String name) {
@@ -991,41 +948,18 @@ public class TableManager extends Producer<TableEvent, TableEventParameters> imp
if (tbl == null) {
dropTblFut.complete(null);
} else {
- EventListener<TableEventParameters> clo = new EventListener<>() {
- @Override
- public boolean notify(@NotNull TableEventParameters parameters, @Nullable Throwable e) {
- String tableName = parameters.tableName();
-
- if (!name.equals(tableName)) {
- return false;
- }
-
- if (e == null) {
- dropTblFut.complete(null);
- } else {
- dropTblFut.completeExceptionally(e);
- }
-
- return true;
- }
-
- @Override
- public void remove(@NotNull Throwable e) {
- dropTblFut.completeExceptionally(e);
- }
- };
-
- listen(TableEvent.DROP, clo);
-
- tablesCfg
- .tables()
+ tablesCfg.tables()
.change(change -> change.delete(name))
- .exceptionally(t -> {
- LOG.error(LoggerMessageHelper.format("Table wasn't dropped [name={}]", name), t);
+ .whenComplete((res, t) -> {
+ if (t != null) {
+ Throwable ex = getRootCause(t);
- removeListener(TableEvent.DROP, clo, new IgniteInternalCheckedException(t.getCause()));
+ LOG.error(LoggerMessageHelper.format("Table wasn't dropped [name={}]", name), ex);
- return null;
+ dropTblFut.completeExceptionally(ex);
+ } else {
+ dropTblFut.complete(res);
+ }
});
}
});