Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: support RPC priority for JDBC connections and statements #1548

Merged
merged 6 commits into from Nov 15, 2021
Merged

feat: support RPC priority for JDBC connections and statements #1548

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4fc1b4b
feat: support RPC priority for JDBC connections and statements
rahul2393 Nov 11, 2021
36152d7
feat: support RPC priority for JDBC connections and statements
rahul2393 Nov 11, 2021
1285647
feat: support RPC priority for JDBC connections and statements
rahul2393 Nov 12, 2021
ba6b4e3
feat: support RPC priority for JDBC connections and statements
rahul2393 Nov 12, 2021
1dbb2d3
feat: support RPC priority for JDBC connections and statements
rahul2393 Nov 12, 2021
fe3d082
feat: support RPC priority for JDBC connections and statements
rahul2393 Nov 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java
View file
Open in desktop
Expand Up @@ -30,6 +30,7 @@ public final class Options implements Serializable {
* set a lower priority for a specific RPC invocation.
*/
public enum RpcPriority {
UNSPECIFIED(Priority.PRIORITY_UNSPECIFIED),
thiagotnunes marked this conversation as resolved.
Show resolved Hide resolved
LOW(Priority.PRIORITY_LOW),
MEDIUM(Priority.PRIORITY_MEDIUM),
HIGH(Priority.PRIORITY_HIGH);
Expand Down
14 changes: 13 additions & 1 deletion ...src/main/java/com/google/cloud/spanner/connection/ClientSideStatementValueConverters.java
View file
Open in desktop
Expand Up @@ -246,8 +246,14 @@ public TransactionMode convert(String value) {
static class RpcPriorityConverter implements ClientSideStatementValueConverter<RpcPriority> {
private final CaseInsensitiveEnumMap<RpcPriority> values =
new CaseInsensitiveEnumMap<>(RpcPriority.class);
private final Pattern allowedValues;

public RpcPriorityConverter(String allowedValues) {}
public RpcPriorityConverter(String allowedValues) {
// Remove the parentheses from the beginning and end.
this.allowedValues =
Pattern.compile(
"(?is)\\A" + allowedValues.substring(1, allowedValues.length() - 1) + "\\z");
}

@Override
public Class<RpcPriority> getParameterClass() {
Expand All @@ -256,6 +262,12 @@ public Class<RpcPriority> getParameterClass() {

@Override
public RpcPriority convert(String value) {
Matcher matcher = allowedValues.matcher(value);
if (matcher.find()) {
if (matcher.group(0).equalsIgnoreCase("null")) {
return RpcPriority.UNSPECIFIED;
}
}
return values.get(value);
}
}
Expand Down
1 change: 0 additions & 1 deletion google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java
View file
Open in desktop
Expand Up @@ -458,7 +458,6 @@ public String getOptimizerStatisticsPackage() {

@Override
public void setRPCPriority(RpcPriority rpcPriority) {
Preconditions.checkNotNull(rpcPriority);
ConnectionPreconditions.checkState(!isClosed(), CLOSED_ERROR_MSG);
this.rpcPriority = rpcPriority;
}
Expand Down
3 changes: 3 additions & 0 deletions ...er/src/main/java/com/google/cloud/spanner/connection/ConnectionStatementExecutorImpl.java
View file
Open in desktop
Expand Up @@ -345,6 +345,9 @@ public StatementResult statementAbortBatch() {

@Override
public StatementResult statementSetRPCPriority(RpcPriority rpcPriority) {
if (rpcPriority == RpcPriority.UNSPECIFIED) {
thiagotnunes marked this conversation as resolved.
Show resolved Hide resolved
rpcPriority = null;
}
getConnection().setRPCPriority(rpcPriority);
return noResult(SET_RPC_PRIORITY);
}
Expand Down
4 changes: 2 additions & 2 deletions ...-spanner/src/main/resources/com/google/cloud/spanner/connection/ClientSideStatements.json
View file
Open in desktop
Expand Up @@ -371,7 +371,7 @@
}
},
{
"name": "SET RPC_PRIORITY = 'HIGH'|'MEDIUM'|'LOW'",
"name": "SET RPC_PRIORITY = 'HIGH'|'MEDIUM'|'LOW'|'NULL'",
"executorName": "ClientSideStatementSetExecutor",
"resultType": "NO_RESULT",
"regex": "(?is)\\A\\s*set\\s+rpc_priority\\s*(?:=)\\s*(.*)\\z",
Expand All @@ -383,7 +383,7 @@
"setStatement": {
"propertyName": "RPC_PRIORITY",
"separator": "=",
"allowedValues": "'(HIGH|MEDIUM|LOW)'",
"allowedValues": "'(HIGH|MEDIUM|LOW|NULL)'",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a new value to allowedValues means that you also need to regenerate the tests so it will also include tests with the new value:

mvn -P generate-test-sql-scripts compile

"converterName": "ClientSideStatementValueConverters$RpcPriorityConverter"
}
}
Expand Down
32 changes: 9 additions & 23 deletions google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionTest.java
View file
Open in desktop
Expand Up @@ -38,6 +38,7 @@
import com.google.spanner.v1.ExecuteSqlRequest.QueryOptions;
import com.google.spanner.v1.RequestOptions;
import java.util.Arrays;
import java.util.Collections;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
Expand Down Expand Up @@ -299,28 +300,6 @@ protected String getBaseUrl() {
return super.getBaseUrl() + ";rpcPriority=MEDIUM";
}

@Test
public void testRPCPriorityAllowedForCommit() {
try (Connection connection = createConnection()) {
try {
connection.commit();
} catch (SpannerException e) {
assertEquals(ErrorCode.FAILED_PRECONDITION, e.getErrorCode());
}
}
}

@Test
public void testRPCPriorityAllowedForRollback() {
try (Connection connection = createConnection()) {
try {
connection.rollback();
} catch (SpannerException e) {
assertEquals(ErrorCode.FAILED_PRECONDITION, e.getErrorCode());
}
}
}

@Test
public void testQuery_RPCPriority() {
try (Connection connection = createConnection()) {
Expand Down Expand Up @@ -382,7 +361,7 @@ public void testPartitionedUpdate_RPCPriority() {
@Test
public void testBatchUpdate_RPCPriority() {
try (Connection connection = createConnection()) {
connection.executeBatchUpdate(Arrays.asList(INSERT_STATEMENT));
connection.executeBatchUpdate(Collections.singleton(INSERT_STATEMENT));
connection.commit();

assertEquals(1, mockSpanner.countRequestsOfType(ExecuteBatchDmlRequest.class));
Expand Down Expand Up @@ -461,6 +440,13 @@ public void testRunBatch_RPCPriority() {
@Test
public void testShowSetRPCPriority() {
try (Connection connection = createConnection()) {
connection.setRPCPriority(null);
try (ResultSet rs =
connection.execute(Statement.of("SHOW VARIABLE RPC_PRIORITY")).getResultSet()) {
assertTrue(rs.next());
assertEquals("PRIORITY_UNSPECIFIED", rs.getString("RPC_PRIORITY"));
assertFalse(rs.next());
}
connection.execute(Statement.of("SET RPC_PRIORITY='LOW'"));
thiagotnunes marked this conversation as resolved.
Show resolved Hide resolved
try (ResultSet rs =
connection.execute(Statement.of("SHOW VARIABLE RPC_PRIORITY")).getResultSet()) {
Expand Down
29 changes: 13 additions & 16 deletions ...d-spanner/src/test/java/com/google/cloud/spanner/connection/RpcPriorityConverterTest.java
View file
Open in desktop
Expand Up @@ -16,11 +16,8 @@

package com.google.cloud.spanner.connection;

import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;

import com.google.cloud.spanner.Options.RpcPriority;
import com.google.cloud.spanner.connection.ClientSideStatementImpl.CompileException;
Expand All @@ -34,20 +31,20 @@ public class RpcPriorityConverterTest {

@Test
public void testConvert() throws CompileException {
String allowedValues = "'(HIGH|MEDIUM|LOW)'";
assertThat(allowedValues, is(notNullValue()));
String allowedValues = "'(HIGH|MEDIUM|LOW|NULL)'";
RpcPriorityConverter converter =
new ClientSideStatementValueConverters.RpcPriorityConverter(allowedValues);
assertThat(converter.convert("high"), is(equalTo(RpcPriority.HIGH)));
assertThat(converter.convert("HIGH"), is(equalTo(RpcPriority.HIGH)));
assertThat(converter.convert("High"), is(equalTo(RpcPriority.HIGH)));
assertEquals(converter.convert("high"), RpcPriority.HIGH);
assertEquals(converter.convert("HIGH"), RpcPriority.HIGH);
assertEquals(converter.convert("High"), RpcPriority.HIGH);

assertThat(converter.convert("medium"), is(equalTo(RpcPriority.MEDIUM)));
assertThat(converter.convert("Low"), is(equalTo(RpcPriority.LOW)));
assertThat(converter.convert("Medium"), is(equalTo(RpcPriority.MEDIUM)));
assertEquals(converter.convert("medium"), RpcPriority.MEDIUM);
assertEquals(converter.convert("Low"), RpcPriority.LOW);
assertEquals(converter.convert("Medium"), RpcPriority.MEDIUM);

assertThat(converter.convert(""), is(nullValue()));
assertThat(converter.convert(" "), is(nullValue()));
assertThat(converter.convert("random string"), is(nullValue()));
assertNull(converter.convert(""));
assertNull(converter.convert(" "));
assertNull(converter.convert("random string"));
assertEquals(converter.convert("NULL"), RpcPriority.UNSPECIFIED);
}
}