[CDAP-21087] Optimize spanner create table operations

[sidhdirenge]

Table creation for Cloud Spanner is currently taking around 5-6 mins. This is adding delays when service pods come up and this in turn increases CDAP instance creation time. The table creation time in case of Cloud SQL is just 3s.

Current logs:

2024-11-13 13:09:52,307 - INFO  [main:i.c.c.m.e.k.StorageMain@69] - Creating storages
.
.
2024-11-13 13:14:21,632 - INFO  [main:i.c.c.m.e.k.StorageMain@113] - Storage creation completed

This PR aims to optimize the spanner table creation.
UpdateDDL statements are costlier in cloud spanner than cloud sql and it is recommended that we should try to batch these statements and execute together rather than individual calls for every create().

Batching is not needed in case of other supported Storage types. So we should not update the current flow for them.

End result: With these changes, table creation time has reduced to 1min, for the pod which does the major heavy lifting. Other pods are up in 20-40s.

Add unit test:

[masoud-io]

Why a list here? DDLs are being created in Spanner class, so having the list here requires these extra methods and it's a bit error-prone.
How about we add two new methods in the StructuredTableAdmin: isBatchCreateRequired() and batchCommit().

So all we need to do here in StoreDefinition is to call tableAdmin.commit() at the end if tableAdmin.isBatchCreateRequired() returns true.

Now in Spanner implementation, create calls will only store DDLs statements in a list, and upon batchCommit(), we execute all those DDLs.

[sidhdirenge]

Thanks for the suggestion Masoud.

I have moved the ddl statement retention logic in Spanner storage impl.
Regarding the create calls, I think we should keep this create & defer create method different.
This was also highlighted in this review comment by Sanket #15743 (comment)

Create() usually gives the perception that the method itself handles the table creation and if we just persist ddl statements here, it is prone to mistakes like batchCommit() not being called later.
If deferCreate() is used, developer will know, we should always call batchCommit().

I have added another method similar to existing updateOrCreate - the new method is named updateOrDeferCreate.
This keeps the method name & its purpose clear.

Please take a look and let me know what you think.

[sonarcloud]

Quality Gate failed

Failed conditions
65.6% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud