Skip to content

Commit

Permalink
JCES-1896 removing internal state isWriteOperation to depend on Reaso…
Browse files Browse the repository at this point in the history
…n.isWrite(), introducing SqlQuery to improve OO impl
  • Loading branch information
afaruga-atlassian committed Mar 10, 2021
1 parent 989c6b0 commit e69f2a9
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ public Optional<RouteDecision> getCause() {
}

/**
* @return True if the database call would always fail on a read replica.
* @return true if the accompanying {@link SqlCall#call()} would fail when run on replica.
*/
public boolean isWrite() {
public boolean mustRunOnMain() {
return reason.isWrite();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,8 @@ public boolean execute(String sql) throws SQLException {
checkClosed();
final RouteDecisionBuilder decisionBuilder;
final Statement statement;
if (sql.startsWith("set")) {
SqlQuery sqlQuery = new SqlQuery(sql);
if (sqlQuery.isSqlSet()) {
decisionBuilder = new RouteDecisionBuilder(READ_OPERATION).sql(sql);
statement = getReadStatement(decisionBuilder);
} else {
Expand Down Expand Up @@ -501,7 +502,7 @@ public long executeLargeUpdate(String sql, String[] columnNames) throws SQLExcep

<T> T execute(final SqlCall<T> call, final RouteDecision routeDecision) throws SQLException {
final T result = databaseCall.call(call, routeDecision);
if (routeDecision.willRunOnMain() && isWriteOperation) {
if (routeDecision.isWrite() && isWriteOperation) {
recordWriteAfterQueryExecution();
}
return result;
Expand Down Expand Up @@ -558,35 +559,21 @@ public Statement getReadStatement(RouteDecisionBuilder decisionBuilder) {
connectionProvider.getStateDecision().ifPresent(decisionBuilder::cause);
return prepareWriteStatement(decisionBuilder);
}
final String sql = decisionBuilder.getSql();
isWriteOperation = sqlFunction.isFunctionCall(sql) || isUpdate(sql) || isDelete(sql);
if (isWriteOperation) {
SqlQuery sqlQuery = new SqlQuery(decisionBuilder.getSql());
if (sqlQuery.isWriteOperation(sqlFunction)) {
decisionBuilder.reason(WRITE_OPERATION);
return prepareWriteStatement(decisionBuilder);
}

if (isSelectForUpdate(sql)) {
if (sqlQuery.isSelectForUpdate()) {
decisionBuilder.reason(LOCK);
return prepareWriteStatement(decisionBuilder);
}

setCurrentStatement(getCurrentStatement() != null ? getCurrentStatement() : readStatement.get(decisionBuilder));
performOperations();
return getCurrentStatement();
}

private boolean isSelectForUpdate(String sql) {
return sql != null && (sql.endsWith("for update") || sql.endsWith("FOR UPDATE"));
}

private boolean isUpdate(String sql) {
return sql != null && (sql.startsWith("update") || sql.startsWith("UPDATE"));
}

private boolean isDelete(String sql) {
return sql != null && (sql.startsWith("delete") || sql.startsWith("DELETE"));
}

protected Statement getWriteStatement(RouteDecisionBuilder decisionBuilder) {
isWriteOperation = true;
return prepareWriteStatement(decisionBuilder);
Expand Down
30 changes: 30 additions & 0 deletions src/main/java/com/atlassian/db/replica/internal/SqlQuery.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package com.atlassian.db.replica.internal;

public final class SqlQuery {

private final String sql;

public SqlQuery(String sql) {
this.sql = sql;
}

boolean isWriteOperation(SqlFunction sqlFunction) {
return sqlFunction.isFunctionCall(sql) || isUpdate() || isDelete();
}

boolean isSelectForUpdate() {
return sql != null && (sql.endsWith("for update") || sql.endsWith("FOR UPDATE"));
}

boolean isSqlSet() {
return sql.startsWith("set");
}

private boolean isUpdate() {
return sql != null && (sql.startsWith("update") || sql.startsWith("UPDATE"));
}

private boolean isDelete() {
return sql != null && (sql.startsWith("delete") || sql.startsWith("DELETE"));
}
}
13 changes: 13 additions & 0 deletions src/test/java/com/atlassian/db/replica/api/TestConsistency.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.sql.PreparedStatement;
import java.sql.SQLException;

import static com.atlassian.db.replica.api.Queries.SELECT_FOR_UPDATE;
import static com.atlassian.db.replica.api.Queries.SIMPLE_QUERY;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.*;
Expand Down Expand Up @@ -105,6 +106,18 @@ public void shouldRefreshAfterFunctionCall() throws SQLException {
verify(consistency).write(any());
}

@Test
public void shouldNotRefreshAfterSelectForUpdate() throws SQLException {
final ConnectionProviderMock connectionProvider = new ConnectionProviderMock();
final ReplicaConsistency consistency = mock(ReplicaConsistency.class);
when(consistency.isConsistent(any())).thenReturn(true);
final Connection connection = DualConnection.builder(connectionProvider, consistency).build();

connection.prepareStatement(SELECT_FOR_UPDATE).executeQuery();

verify(consistency, never()).write(any());
}

@Test
public void shouldSetAutoCommitRefreshWhenInTransaction() throws SQLException {
final ConnectionProviderMock connectionProvider = new ConnectionProviderMock();
Expand Down

0 comments on commit e69f2a9

Please sign in to comment.