Skip to content

Commit 5b1df19

Browse files
committed
Add IT for repairCoordinatorTables Coordinator schema upgrade
Verifies that calling repairCoordinatorTables with scalar.db.consensus_commit.coordinator.group_commit.enabled toggled from false to true against an existing pre-group-commit Coordinator table issues ALTER TABLE ADD COLUMN to add the child_ids column.
1 parent af66079 commit 5b1df19

2 files changed

Lines changed: 66 additions & 1 deletion

File tree

core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdmin.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,8 +350,16 @@ public void repairCoordinatorTables(Map<String, String> options) throws Executio
350350
coordinatorTableMetadata = getCoordinatorTableMetadata();
351351
}
352352

353-
admin.repairTable(coordinatorNamespace, Coordinator.TABLE, coordinatorTableMetadata, options);
353+
// Upgrade the schema (ALTER TABLE ADD COLUMN for any non-key columns the desired schema
354+
// requires that the existing Coordinator is missing) BEFORE repairTable. addNewColumnToTable
355+
// checks the existing metadata via getTableMetadata, so it must run while the metadata still
356+
// reflects the pre-upgrade column set. If repairTable runs first, it upserts the metadata to
357+
// the desired schema (which already includes the column), and the subsequent
358+
// addNewColumnToTable call would be rejected with "column already exists" against the
359+
// ScalarDB-side metadata even though the physical column does not yet exist.
354360
upgradeCoordinatorTableSchema(currentMetadata, coordinatorTableMetadata);
361+
362+
admin.repairTable(coordinatorNamespace, Coordinator.TABLE, coordinatorTableMetadata, options);
355363
}
356364

357365
// Adds any non-key columns the desired Coordinator table schema requires that the existing

integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdminRepairTableIntegrationTestBase.java

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,16 @@
11
package com.scalar.db.transaction.consensuscommit;
22

3+
import static org.assertj.core.api.Assertions.assertThat;
4+
5+
import com.scalar.db.api.DistributedStorageAdmin;
6+
import com.scalar.db.api.DistributedTransactionAdmin;
37
import com.scalar.db.api.DistributedTransactionAdminRepairTableIntegrationTestBase;
8+
import com.scalar.db.api.TableMetadata;
9+
import com.scalar.db.config.DatabaseConfig;
10+
import com.scalar.db.service.StorageFactory;
11+
import com.scalar.db.service.TransactionFactory;
412
import java.util.Properties;
13+
import org.junit.jupiter.api.Test;
514

615
public abstract class ConsensusCommitAdminRepairTableIntegrationTestBase
716
extends DistributedTransactionAdminRepairTableIntegrationTestBase {
@@ -18,4 +27,52 @@ protected final Properties getProperties(String testName) {
1827
}
1928

2029
protected abstract Properties getProps(String testName);
30+
31+
@Test
32+
public void
33+
repairCoordinatorTables_WithExistingCoordinatorMissingChildIdsColumn_ShouldAddChildIdsColumn()
34+
throws Exception {
35+
// Arrange: drop the Coordinator created in setUp and recreate it via a separate admin
36+
// instance that has group commit explicitly disabled, so the Coordinator table has no
37+
// CHILD_IDS column.
38+
admin.dropCoordinatorTables();
39+
40+
Properties propertiesWithGroupCommitDisabled = new Properties();
41+
propertiesWithGroupCommitDisabled.putAll(getProperties(TEST_NAME));
42+
propertiesWithGroupCommitDisabled.setProperty(
43+
ConsensusCommitConfig.COORDINATOR_GROUP_COMMIT_ENABLED, "false");
44+
try (DistributedTransactionAdmin adminWithGroupCommitDisabled =
45+
TransactionFactory.create(propertiesWithGroupCommitDisabled).getTransactionAdmin()) {
46+
waitForDifferentSessionDdl();
47+
adminWithGroupCommitDisabled.createCoordinatorTables(getCreationOptions());
48+
}
49+
50+
// Act: repair with group commit enabled. The existing Coordinator has no CHILD_IDS column,
51+
// so this should add the column via ALTER TABLE ADD COLUMN.
52+
Properties propertiesWithGroupCommitEnabled = new Properties();
53+
propertiesWithGroupCommitEnabled.putAll(getProperties(TEST_NAME));
54+
propertiesWithGroupCommitEnabled.setProperty(
55+
ConsensusCommitConfig.COORDINATOR_GROUP_COMMIT_ENABLED, "true");
56+
try (DistributedTransactionAdmin adminWithGroupCommitEnabled =
57+
TransactionFactory.create(propertiesWithGroupCommitEnabled).getTransactionAdmin()) {
58+
waitForDifferentSessionDdl();
59+
adminWithGroupCommitEnabled.repairCoordinatorTables(getCreationOptions());
60+
61+
// Assert: the column now exists in the post-repair Coordinator metadata. We have to read the
62+
// metadata via DistributedStorageAdmin (not via the transaction admin) because
63+
// ConsensusCommitAdmin#getTableMetadata returns null for the coordinator namespace by design.
64+
waitForDifferentSessionDdl();
65+
String coordinatorNamespace =
66+
new ConsensusCommitConfig(new DatabaseConfig(propertiesWithGroupCommitEnabled))
67+
.getCoordinatorNamespace()
68+
.orElse(Coordinator.NAMESPACE);
69+
try (DistributedStorageAdmin storageAdmin =
70+
StorageFactory.create(propertiesWithGroupCommitEnabled).getStorageAdmin()) {
71+
TableMetadata metadata =
72+
storageAdmin.getTableMetadata(coordinatorNamespace, Coordinator.TABLE);
73+
assertThat(metadata).isNotNull();
74+
assertThat(metadata.getColumnNames()).contains(Attribute.CHILD_IDS);
75+
}
76+
}
77+
}
2178
}

0 commit comments

Comments
 (0)