Supporting concurrent schema update

[officialtushark]

Query engine

No response

Question

Does Iceberg support concurrent schema update for the same table?

I have two transaction, namely t1 and t2.
The transaction t1 has following new columns added to it {col1:int,col2:int}
The transaction t2 has following new columns added to it {col1:int}

In my test case, the transaction t2 fails if t1 is committed earlier. This is the stack trace for the failure:

org.apache.iceberg.exceptions.CommitFailedException: Table metadata refresh is required
	at org.apache.iceberg.BaseTransaction$TransactionTableOperations.commit(BaseTransaction.java:545)
	at org.apache.iceberg.SchemaUpdate.commit(SchemaUpdate.java:442)
	at org.apache.iceberg.BaseTransaction.applyUpdates(BaseTransaction.java:497)
	at org.apache.iceberg.BaseTransaction.lambda$commitSimpleTransaction$5(BaseTransaction.java:420)
	at org.apache.iceberg.util.Tasks$Builder.runTaskWithRetry(Tasks.java:413)
	at org.apache.iceberg.util.Tasks$Builder.runSingleThreaded(Tasks.java:219)
	at org.apache.iceberg.util.Tasks$Builder.run(Tasks.java:203)
	at org.apache.iceberg.util.Tasks$Builder.run(Tasks.java:196)
	at org.apache.iceberg.BaseTransaction.commitSimpleTransaction(BaseTransaction.java:418)
	at org.apache.iceberg.BaseTransaction.commitTransaction(BaseTransaction.java:302)

Looking under the hood, I found that Iceberg checks if the current metadata is same as the base metadata, if not then it throws this exception, essentially failing the transaction.

I could not find a way to update the base metadata of the pending updates in a transaction.

Is there a way to make these concurrent updates work?

[nastra]

This is in fact expected behavior. There's also a test that tests exactly this use case:

iceberg/core/src/test/java/org/apache/iceberg/TestTransaction.java

Lines 421 to 445 in 7c3c348

	public void testTransactionRetrySchemaUpdate() {
	// use only one retry
	table.updateProperties().set(TableProperties.COMMIT_NUM_RETRIES, "1").commit();
	// start a transaction
	Transaction txn = table.newTransaction();
	// add column "new-column"
	txn.updateSchema().addColumn("new-column", Types.IntegerType.get()).commit();
	int schemaId = txn.table().schema().schemaId();
	// directly update the table for adding "another-column" (which causes in-progress txn commit
	// fail)
	table.updateSchema().addColumn("another-column", Types.IntegerType.get()).commit();
	int conflictingSchemaId = table.schema().schemaId();
	Assert.assertEquals(
	"Both schema IDs should be the same in order to cause a conflict",
	conflictingSchemaId,
	schemaId);
	// commit the transaction for adding "new-column"
	Assertions.assertThatThrownBy(txn::commitTransaction)
	.isInstanceOf(CommitFailedException.class)
	.hasMessage("Table metadata refresh is required");
	}

[officialtushark]

Thanks @nastra. I did see this test case demonstrating the behaviour. However I could not wrap my head around the reasoning for this.

At one hand, I do understand a conflicting schema change (t1 adds a col1:int and t2 adds a col1:double) should trigger a failure . However, In case when there is non-conflicting change to the schema(t1 add col1:int, t2 also adds col1:int), why do we resort to fail the transaction?

[RussellSpitzer]

Just not an implemented validation check yet. Pull requests are welcome :)

[officialtushark]

Got it @RussellSpitzer . I am open to submit a PR for this check. However this snippet would always be true in case we are doing a schema update and some other transaction committed before. Do you have some recommendation around the implementation of this feature?

https://github.com/apache/iceberg/blob/00f771de3a71c4de39e310114f0f776c7a81838d/core/src/main/java/org/apache/iceberg/BaseMetastoreTableOperations.java#L118C1-L127C6

[RussellSpitzer]

It would be a rather large change, we probably would need some sort of validation logic which all operations were able to subclass and change based on what they want to do. I know this ends up looking a lot simpler in the REST catalog where we don't have the whole "metadata" to compare and instead we have a set of changes.

[officialtushark]

Yes I agree, I potentially see some logic changes(upgrades?) inside core. This involves two sub-part I guess then:

1. Deciding the scope of this feature (which types of operations are going to be considered as non-conflicting and conflicting)
2. Implementation of the same

Should I open a separate issue for this? I can work on a doc outlining both of the above sub-parts and once we agree on the semantics,I can work on a PR to integrate this feature.

[RussellSpitzer]

I would think we would nee a pretty extensive design doc if we are modifying core since this will interact with a lot of things. That's probably a good starting place, you may also just want to raise this on the dev list to see if there is actually a lot of interest in making these modifications. They will necessitate changing some very basic things so there will be a rather large surface area.

[officialtushark]

Thanks Russell. Sure, Let me raise this on the dev list.