Skip to content

Commit

Permalink
[fix](query-forward) Fix forward query exception or stuck or potentia…
Browse files Browse the repository at this point in the history
…l query result loss (apache#41303)

1. Fix forward query exception if no status code is set in master
execution. EOF may result in this status.

2. Fix forward query stuck due to no result packet sent to mysql
channel. Should use result packets from master.

3. Fix potential forward query result loss if follower can read status
change during query process. Should judge by the status once before
execution.

4. Add assertion for regression test.
  • Loading branch information
TangSiyang2001 committed Oct 24, 2024
1 parent 7b5a43e commit ca3ae64
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3137,4 +3137,9 @@ public static int metaServiceRpcRetryTimes() {
@ConfField(mutable = true, description = {"元数据同步是否开启安全模式",
"Is metadata synchronization enabled in safe mode"})
public static boolean meta_helper_security_mode = false;

@ConfField(description = {"用于测试,强制将所有的查询forward到master以验证forward query的行为",
"For testing purposes, all queries are forcibly forwarded to the master to verify"
+ "the behavior of forwarding queries."})
public static boolean force_forward_all_queries = false;
}
Original file line number Diff line number Diff line change
Expand Up @@ -560,11 +560,8 @@ public void finalizeCommand() throws IOException {
&& ctx.getState().getStateType() != QueryState.MysqlStateType.ERR) {
ShowResultSet resultSet = executor.getShowResultSet();
if (resultSet == null) {
if (executor.sendProxyQueryResult()) {
packet = getResultPacket();
} else {
packet = executor.getOutputPacket();
}
executor.sendProxyQueryResult();
packet = executor.getOutputPacket();
} else {
executor.sendResultSet(resultSet);
packet = getResultPacket();
Expand Down Expand Up @@ -724,7 +721,12 @@ public TMasterOpResult proxyExecute(TMasterOpRequest request) throws TException
if (ctx.getState().getStateType() == MysqlStateType.OK) {
result.setStatusCode(0);
} else {
result.setStatusCode(ctx.getState().getErrorCode().getCode());
ErrorCode errorCode = ctx.getState().getErrorCode();
if (errorCode != null) {
result.setStatusCode(errorCode.getCode());
} else {
result.setStatusCode(ErrorCode.ERR_UNKNOWN_ERROR.getCode());
}
result.setErrMessage(ctx.getState().getErrorMessage());
}
if (request.isSetTxnLoadInfo()) {
Expand Down
20 changes: 12 additions & 8 deletions fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ public class StmtExecutor {
private boolean isHandleQueryInFe = false;
// The profile of this execution
private final Profile profile;
private Boolean isForwardedToMaster = null;

// The result schema if "dry_run_query" is true.
// Only one column to indicate the real return row numbers.
Expand Down Expand Up @@ -423,6 +424,13 @@ public void setPlanner(Planner planner) {
}

public boolean isForwardToMaster() {
if (isForwardedToMaster == null) {
isForwardedToMaster = shouldForwardToMaster();
}
return isForwardedToMaster;
}

private boolean shouldForwardToMaster() {
if (Env.getCurrentEnv().isMaster()) {
return false;
}
Expand All @@ -433,7 +441,7 @@ public boolean isForwardToMaster() {

// this is a query stmt, but this non-master FE can not read, forward it to master
if (isQuery() && !Env.getCurrentEnv().isMaster()
&& (!Env.getCurrentEnv().canRead() || debugForwardAllQueries())) {
&& (!Env.getCurrentEnv().canRead() || debugForwardAllQueries() || Config.force_forward_all_queries)) {
return true;
}

Expand All @@ -446,7 +454,7 @@ public boolean isForwardToMaster() {

private boolean debugForwardAllQueries() {
DebugPoint debugPoint = DebugPointUtil.getDebugPoint("StmtExecutor.forward_all_queries");
return debugPoint != null && debugPoint.param("forwardAllQueries", true);
return debugPoint != null && debugPoint.param("forwardAllQueries", false);
}

public ByteBuffer getOutputPacket() {
Expand Down Expand Up @@ -3618,17 +3626,13 @@ public List<ByteBuffer> getProxyQueryResultBufList() {
return ((ProxyMysqlChannel) context.getMysqlChannel()).getProxyResultBufferList();
}

public boolean sendProxyQueryResult() throws IOException {
public void sendProxyQueryResult() throws IOException {
if (masterOpExecutor == null) {
return false;
return;
}
List<ByteBuffer> queryResultBufList = masterOpExecutor.getQueryResultBufList();
if (queryResultBufList.isEmpty()) {
return false;
}
for (ByteBuffer byteBuffer : queryResultBufList) {
context.getMysqlChannel().sendOnePacket(byteBuffer);
}
return true;
}
}
5 changes: 3 additions & 2 deletions regression-test/suites/query_p0/test_forward_qeury.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ suite("test_forward_query", 'docker') {

sql """ INSERT INTO ${tbl} VALUES(1);"""

cluster.injectDebugPoints(NodeType.FE, ['StmtExecutor.forward_all_queries' : [forwardAllQueries:true]])
cluster.injectDebugPoints(NodeType.FE, ['StmtExecutor.forward_all_queries' : [forwardAllQueries:true, execute:1]])

sql """ SELECT * FROM ${tbl} """
def ret = sql """ SELECT * FROM ${tbl} """
assertEquals(ret[0][0], 1)
}
}

0 comments on commit ca3ae64

Please sign in to comment.