Copilot commented on code in PR #63721:
URL: https://github.com/apache/doris/pull/63721#discussion_r3308866121
##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -612,6 +617,35 @@ public void queryRetry(TUniqueId queryId) throws Exception
{
}
}
+ boolean shouldDisableCloudVersionCacheOnRetry(String errorMessage) {
+ return Config.isCloudMode()
+ && errorMessage != null
+ && errorMessage.contains(SystemInfoService.ERROR_E230)
+ &&
(context.getSessionVariable().cloudPartitionVersionCacheTtlMs != 0
+ || context.getSessionVariable().cloudTableVersionCacheTtlMs !=
0);
+ }
+
+ void setCloudVersionCacheTtlZeroForNextAttempt() {
+ SessionVariable sessionVariable = context.getSessionVariable();
+ long partitionTtl = sessionVariable.cloudPartitionVersionCacheTtlMs;
+ long tableTtl = sessionVariable.cloudTableVersionCacheTtlMs;
+
+ if (partitionTtl != 0
+ &&
!sessionVariable.setVarOnce(SessionVariable.CLOUD_PARTITION_VERSION_CACHE_TTL_MS,
"0")) {
+ LOG.warn("failed to set {}=0 before retry. {}",
+ SessionVariable.CLOUD_PARTITION_VERSION_CACHE_TTL_MS,
context.getQueryIdentifier());
+ }
+ if (tableTtl != 0
+ &&
!sessionVariable.setVarOnce(SessionVariable.CLOUD_TABLE_VERSION_CACHE_TTL_MS,
"0")) {
+ LOG.warn("failed to set {}=0 before retry. {}",
+ SessionVariable.CLOUD_TABLE_VERSION_CACHE_TTL_MS,
context.getQueryIdentifier());
+ }
+ LOG.info("temporarily set {} from {} to 0 and {} from {} to 0 before
retry. {}",
+ SessionVariable.CLOUD_PARTITION_VERSION_CACHE_TTL_MS,
partitionTtl,
+ SessionVariable.CLOUD_TABLE_VERSION_CACHE_TTL_MS, tableTtl,
+ context.getQueryIdentifier());
Review Comment:
The INFO log unconditionally states both TTLs were set to 0, but
`setVarOnce(...)` can fail (you already warn on failure). This can produce
misleading INFO logs; consider logging success flags or only logging the change
when the set actually succeeds.
##########
regression-test/suites/cloud_p0/query_retry/test_retry_e-230.groovy:
##########
@@ -162,7 +162,62 @@ suite("test_retry_e-230", 'docker') {
// fe StmtExecutor retry time, at most 25 * 1.5s + 25 * 2.5s
assertTrue(cost > 4000 && cost < 100000)
+ def restoreTbl = 'test_retry_e_230_restore_tbl'
+ sql """ DROP TABLE IF EXISTS ${restoreTbl} """
+ sql """
+ CREATE TABLE ${restoreTbl} (
+ `k1` int(11) NULL,
+ `k2` int(11) NULL
+ )
+ DUPLICATE KEY(`k1`, `k2`)
+ COMMENT 'OLAP'
+ DISTRIBUTED BY HASH(`k1`) BUCKETS 1
+ PROPERTIES (
+ "replication_num"="1"
+ );
+ """
+ for (def i = 1; i <= 3; i++) {
+ insert_sql """INSERT INTO ${restoreTbl} VALUES (${i},
${100 * i})""", 1
+ }
+
+ sql """ set cloud_partition_version_cache_ttl_ms = 3600000 """
+ sql """ set cloud_table_version_cache_ttl_ms = 3600000 """
+ def row_cnt = sql """select count() from ${restoreTbl}"""
+ assertEquals(row_cnt[0][0], 3)
+
+ cluster.injectDebugPoints(NodeType.BE,
['CloudTablet.capture_rs_readers.return.e-230' : null])
+ cluster.injectDebugPoints(NodeType.FE,
['StmtExecutor.retry.longtime' : null])
+ insert_sql """INSERT INTO ${restoreTbl} VALUES (4, 400)""", 1
+
+ def futrue5 = thread {
+ Thread.sleep(4000)
+ cluster.clearBackendDebugPoints()
+ }
+
+ begin = System.currentTimeMillis();
+ def futrue6 = thread {
+ def result = sql """select * from ${restoreTbl} order by
k1"""
+ log.info("select result: {}", result)
+ }
+
+ futrue6.get()
+ cost = System.currentTimeMillis() - begin;
+ log.info("time cost restore check : {}", cost)
+ futrue5.get()
+ assertTrue(cost > 4000 && cost < 100000)
Review Comment:
The new thread futures are named `futrue5`/`futrue6` (typo). Please rename
them (and their `.get()` usages) to `future*` (or a descriptive name) to
improve readability/maintainability of the test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
