delta icon indicating copy to clipboard operation
delta copied to clipboard

Published 20 hours ago • verified publisher icon delta-io

Enable WriteSerializable Isolation Level

Open koertkuipers opened this issue 3 years ago • 7 comments

Description

Resolves #1261

How was this patch tested?

Added tests for isolation level WriteSerializable in OptimisticTransactionSuite

Does this PR introduce any user-facing changes?

Yes. it enabled setting config delta.isolationLevel=WriteSerializable

koertkuipers avatar Jul 09 '22 19:07 koertkuipers

this pullreq is WIP because i am waiting to hear if there is any good reason this should not be exposed...

koertkuipers avatar Jul 12 '22 03:07 koertkuipers

ok i will create new pullreq to just expose isolation level and only allow Serializable. i will let this one remain WIP just in case it turns out current code for WriteSerializable is correct.

koertkuipers avatar Jul 16 '22 15:07 koertkuipers

ok i will create new pullreq to just expose isolation level and only allow Serializable. i will let this one remain WIP just in case it turns out current code for WriteSerializable is correct.

What's the status here? Did you create that new PR? Can we close this one?

Thanks!

scottsand-db avatar Sep 20 '22 18:09 scottsand-db

ok i will create new pullreq to just expose isolation level and only allow Serializable. i will let this one remain WIP just in case it turns out current code for WriteSerializable is correct.

What's the status here? Did you create that new PR? Can we close this one?

Thanks!

the pullrequest to enable just setting Serializable isolation level has been merged. it fixes https://github.com/delta-io/delta/issues/1265

this pullrequest enables WriteSerializable isolation level, which is a separate issue that has not been resolved yet. see https://github.com/delta-io/delta/issues/1261 i am waiting for @zsxwing to take a detailed look at current code for WriteSerializable which this pullreq depends on (and thats why its marked as [WIP]).

koertkuipers avatar Oct 15 '22 15:10 koertkuipers

@zsxwing can you take a look please?

this passes all tests without issues, and we have been running this for years now on clusters (HDFS and S3). the logic for WriteSerializable in ConflictChecker seems sound to me (but i am not the expert on this...)

WriteSerializable is the far more useful isolation level (and the default in databricks platform). without WriteSerializable isolation even the most basic concurrent writes will fail. allowing only Serializable isolation is making open source delta a bit of a second class citizen, so i think this is high priority.

koertkuipers avatar May 17 '23 17:05 koertkuipers

Is this still a WIP?

felipepessoto avatar Jul 25 '23 20:07 felipepessoto

Is this still a WIP?

i think it is not. let me change title. i does still need to be reviewed though by people that understand this part of codebase.

koertkuipers avatar Jul 26 '23 23:07 koertkuipers