-
Notifications
You must be signed in to change notification settings - Fork 132
feat: support isolation level REPEATABLE_READ for R/W transactions #3670
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
23494f5
cd8ee25
97bcd9f
2df96f9
d63e36e
afdf0eb
12f8a93
515db9c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,9 +21,13 @@ | |
import com.google.spanner.v1.ReadRequest.LockHint; | ||
import com.google.spanner.v1.ReadRequest.OrderBy; | ||
import com.google.spanner.v1.RequestOptions.Priority; | ||
import com.google.spanner.v1.TransactionOptions.IsolationLevel; | ||
import java.io.Serializable; | ||
import java.time.Duration; | ||
import java.util.Arrays; | ||
import java.util.Objects; | ||
import java.util.function.Predicate; | ||
import java.util.stream.Stream; | ||
|
||
/** Specifies options for various spanner operations */ | ||
public final class Options implements Serializable { | ||
|
@@ -131,7 +135,29 @@ public interface UpdateAdminApiOption extends AdminApiOption {} | |
public interface QueryOption {} | ||
|
||
/** Marker interface to mark options applicable to write operations */ | ||
public interface TransactionOption {} | ||
shobhitsg marked this conversation as resolved.
Show resolved
Hide resolved
|
||
public interface TransactionOption { | ||
Predicate<TransactionOption> isValidIsolationLevelOption = | ||
txnOption -> | ||
txnOption instanceof RepeatableReadOption || txnOption instanceof SerializableOption; | ||
|
||
/** | ||
* Combines two arrays of TransactionOption, with primaryOptions taking precedence in case of | ||
* conflicts. Note that {@link | ||
* com.google.cloud.spanner.SpannerOptions.Builder.TransactionOptions} supports only the {@link | ||
* IsolationLevel} TransactionOption, meaning spannerOptions will contain a maximum of one | ||
* TransactionOption. | ||
*/ | ||
static TransactionOption[] combine( | ||
TransactionOption[] primaryOptions, TransactionOption[] spannerOptions) { | ||
if (spannerOptions == null | ||
|| Arrays.stream(primaryOptions).anyMatch(isValidIsolationLevelOption)) { | ||
return primaryOptions; | ||
} else { | ||
return Stream.concat(Arrays.stream(primaryOptions), Arrays.stream(spannerOptions)) | ||
.toArray(TransactionOption[]::new); | ||
} | ||
} | ||
} | ||
shobhitsg marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/** Marker interface to mark options applicable to update operation. */ | ||
public interface UpdateOption {} | ||
|
@@ -159,6 +185,22 @@ public static TransactionOption optimisticLock() { | |
return OPTIMISTIC_LOCK_OPTION; | ||
} | ||
|
||
/** | ||
* Specifying this instructs the transaction to request Repeatable Read Isolation Level from the | ||
* backend. | ||
*/ | ||
public static TransactionOption repeatableReadIsolationLevel() { | ||
return REPEATABLE_READ_OPTION; | ||
} | ||
|
||
/** | ||
* Specifying this instructs the transaction to request Serializable Isolation Level from the | ||
* backend. | ||
*/ | ||
public static TransactionOption serializableIsolationLevel() { | ||
return SERIALIZABLE_OPTION; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of creating two methods, one for each isolation level, this should be one method that takes an IsolationLevel input argument. This means that any future isolation levels that we add can be used without any changes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I completely agree with your point. However, IMO, including request options that are not yet fully supported(when we add more IsolationLevel) by the library, even for short duration(e.g. development if that's even required?), can create confusion among lib users. Though the backend will reject unsupported options, their presence without clear documentation could mislead developers who often rely on request options(as it's a valid contract) to determine functionality. Please let me know if I've misunderstood anything and I'd like to know your POV. Here, I've assumed that the proto will always be merged in the main branch before any development could start. |
||
|
||
/** | ||
* Specifying this instructs the transaction to be excluded from being recorded in change streams | ||
* with the DDL option `allow_txn_exclusion=true`. This does not exclude the transaction from | ||
|
@@ -490,6 +532,26 @@ void appendToOptions(Options options) { | |
} | ||
} | ||
|
||
/** Option to request REPEATABLE READ isolation level for read/write transactions. */ | ||
static final class RepeatableReadOption extends InternalOption implements TransactionOption { | ||
@Override | ||
void appendToOptions(Options options) { | ||
options.isolationLevel = IsolationLevel.REPEATABLE_READ; | ||
} | ||
} | ||
|
||
static final RepeatableReadOption REPEATABLE_READ_OPTION = new RepeatableReadOption(); | ||
|
||
/** Option to request SERIALIZABLE isolation level for read/write transactions. */ | ||
static final class SerializableOption extends InternalOption implements TransactionOption { | ||
@Override | ||
void appendToOptions(Options options) { | ||
options.isolationLevel = IsolationLevel.SERIALIZABLE; | ||
} | ||
} | ||
|
||
static final SerializableOption SERIALIZABLE_OPTION = new SerializableOption(); | ||
shobhitsg marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
private boolean withCommitStats; | ||
|
||
private Duration maxCommitDelay; | ||
|
@@ -512,6 +574,7 @@ void appendToOptions(Options options) { | |
private RpcOrderBy orderBy; | ||
private RpcLockHint lockHint; | ||
private Boolean lastStatement; | ||
private IsolationLevel isolationLevel; | ||
|
||
// Construction is via factory methods below. | ||
private Options() {} | ||
|
@@ -664,6 +727,10 @@ LockHint lockHint() { | |
return lockHint == null ? null : lockHint.proto; | ||
} | ||
|
||
IsolationLevel isolationLevel() { | ||
return isolationLevel; | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
StringBuilder b = new StringBuilder(); | ||
|
@@ -726,6 +793,9 @@ public String toString() { | |
if (lockHint != null) { | ||
b.append("lockHint: ").append(lockHint).append(' '); | ||
} | ||
if (isolationLevel != null) { | ||
b.append("isolationLevel: ").append(isolationLevel).append(' '); | ||
} | ||
return b.toString(); | ||
} | ||
|
||
|
@@ -767,7 +837,8 @@ public boolean equals(Object o) { | |
&& Objects.equals(directedReadOptions(), that.directedReadOptions()) | ||
&& Objects.equals(orderBy(), that.orderBy()) | ||
&& Objects.equals(isLastStatement(), that.isLastStatement()) | ||
&& Objects.equals(lockHint(), that.lockHint()); | ||
&& Objects.equals(lockHint(), that.lockHint()) | ||
&& Objects.equals(isolationLevel(), that.isolationLevel()); | ||
} | ||
|
||
@Override | ||
|
@@ -833,6 +904,9 @@ public int hashCode() { | |
if (lockHint != null) { | ||
result = 31 * result + lockHint.hashCode(); | ||
} | ||
if (isolationLevel != null) { | ||
result = 31 * result + isolationLevel.hashCode(); | ||
} | ||
return result; | ||
} | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.