chore: update executor to fix a SI issue#13492
Conversation
There was a problem hiding this comment.
Code Review
This pull request modifies the transaction lock mode configuration in CloudClientExecutor.java based on repeatableRead and optimistic flags. The reviewer pointed out that since PESSIMISTIC is the default lock mode in Spanner, the logic can be simplified to only explicitly set the OPTIMISTIC lock mode when necessary, and provided a code suggestion to achieve this.
| if (!repeatableRead && optimistic) { | ||
| transactionOptions.add(Options.readLockMode(ReadLockMode.OPTIMISTIC)); | ||
| } else { | ||
| } | ||
| if (repeatableRead && !optimistic) { | ||
| transactionOptions.add(Options.readLockMode(ReadLockMode.PESSIMISTIC)); | ||
| } |
There was a problem hiding this comment.
The logic for setting the transaction lock mode can be simplified. The default lock mode for read-write transactions in Spanner is PESSIMISTIC. Therefore, you only need to explicitly set the lock mode when OPTIMISTIC is required.
The current implementation has a redundant condition that explicitly sets the PESSIMISTIC lock mode, which is already the default.
You can simplify this block to a single if statement that handles the OPTIMISTIC case, making the code more concise and readable.
| if (!repeatableRead && optimistic) { | |
| transactionOptions.add(Options.readLockMode(ReadLockMode.OPTIMISTIC)); | |
| } else { | |
| } | |
| if (repeatableRead && !optimistic) { | |
| transactionOptions.add(Options.readLockMode(ReadLockMode.PESSIMISTIC)); | |
| } | |
| if (optimistic && !repeatableRead) { | |
| transactionOptions.add(Options.readLockMode(ReadLockMode.OPTIMISTIC)); | |
| } |
No description provided.