github-actions[bot] commented on code in PR #65413:
URL: https://github.com/apache/doris/pull/65413#discussion_r3550173068
##########
regression-test/suites/compaction/test_base_compaction_with_dup_key_max_file_size_limit.groovy:
##########
@@ -65,27 +65,48 @@
suite("test_base_compaction_with_dup_key_max_file_size_limit", "p2") {
}
}
}
- try {
- String backend_id;
- def backendId_to_backendIP = [:]
- def backendId_to_backendHttpPort = [:]
- getBackendIpHttpPort(backendId_to_backendIP,
backendId_to_backendHttpPort);
-
- backend_id = backendId_to_backendIP.keySet()[0]
- def (code, out, err) =
show_be_config(backendId_to_backendIP.get(backend_id),
backendId_to_backendHttpPort.get(backend_id))
-
- logger.info("Show config: code=" + code + ", out=" + out + ", err=" +
err)
- assertEquals(code, 0)
- def configList = parseJson(out.trim())
- assert configList instanceof List
-
- boolean disableAutoCompaction = true
- for (Object ele in (List) configList) {
- assert ele instanceof List<String>
- if (((List<String>) ele)[0] == "disable_auto_compaction") {
- disableAutoCompaction = Boolean.parseBoolean(((List<String>)
ele)[2])
- }
+ String backend_id;
+ def backendId_to_backendIP = [:]
+ def backendId_to_backendHttpPort = [:]
+ getBackendIpHttpPort(backendId_to_backendIP, backendId_to_backendHttpPort);
+
+ def set_be_param = { paramName, paramValue ->
+ for (String id in backendId_to_backendIP.keySet()) {
+ def beIp = backendId_to_backendIP.get(id)
+ def bePort = backendId_to_backendHttpPort.get(id)
+ def (rcode, rout, rerr) = curl("POST",
String.format("http://%s:%s/api/update_config?%s=%s";, beIp, bePort, paramName,
paramValue))
+ assertTrue(rout.contains("OK"))
+ }
+ }
+
+ backend_id = backendId_to_backendIP.keySet()[0]
+ def (code, out, err) =
show_be_config(backendId_to_backendIP.get(backend_id),
backendId_to_backendHttpPort.get(backend_id))
+
+ logger.info("Show config: code=" + code + ", out=" + out + ", err=" + err)
+ assertEquals(code, 0)
+ def configList = parseJson(out.trim())
+ assert configList instanceof List
+
+ boolean originalDisableAutoCompaction = false
Review Comment:
This snapshots the original value from only
`backendId_to_backendIP.keySet()[0]`, but `set_be_param` applies the restore to
every backend. Since `/api/update_config` is sent to each BE separately, the
runtime value is per backend; if one BE starts with a different
`disable_auto_compaction` or `base_compaction_dup_key_max_file_size_mbytes`,
the `finally` block overwrites it with BE0's value instead of restoring it.
Nearby compaction tests keep a `backendId_to_params` map keyed by backend id
for this reason. Please capture both original config values per backend and
restore each BE's own values.
##########
regression-test/suites/compaction/test_base_compaction_with_dup_key_max_file_size_limit.groovy:
##########
@@ -65,27 +65,48 @@
suite("test_base_compaction_with_dup_key_max_file_size_limit", "p2") {
}
}
}
- try {
- String backend_id;
- def backendId_to_backendIP = [:]
- def backendId_to_backendHttpPort = [:]
- getBackendIpHttpPort(backendId_to_backendIP,
backendId_to_backendHttpPort);
-
- backend_id = backendId_to_backendIP.keySet()[0]
- def (code, out, err) =
show_be_config(backendId_to_backendIP.get(backend_id),
backendId_to_backendHttpPort.get(backend_id))
-
- logger.info("Show config: code=" + code + ", out=" + out + ", err=" +
err)
- assertEquals(code, 0)
- def configList = parseJson(out.trim())
- assert configList instanceof List
-
- boolean disableAutoCompaction = true
- for (Object ele in (List) configList) {
- assert ele instanceof List<String>
- if (((List<String>) ele)[0] == "disable_auto_compaction") {
- disableAutoCompaction = Boolean.parseBoolean(((List<String>)
ele)[2])
- }
+ String backend_id;
+ def backendId_to_backendIP = [:]
+ def backendId_to_backendHttpPort = [:]
+ getBackendIpHttpPort(backendId_to_backendIP, backendId_to_backendHttpPort);
+
+ def set_be_param = { paramName, paramValue ->
+ for (String id in backendId_to_backendIP.keySet()) {
+ def beIp = backendId_to_backendIP.get(id)
+ def bePort = backendId_to_backendHttpPort.get(id)
+ def (rcode, rout, rerr) = curl("POST",
String.format("http://%s:%s/api/update_config?%s=%s";, beIp, bePort, paramName,
paramValue))
+ assertTrue(rout.contains("OK"))
+ }
+ }
+
+ backend_id = backendId_to_backendIP.keySet()[0]
+ def (code, out, err) =
show_be_config(backendId_to_backendIP.get(backend_id),
backendId_to_backendHttpPort.get(backend_id))
+
+ logger.info("Show config: code=" + code + ", out=" + out + ", err=" + err)
+ assertEquals(code, 0)
+ def configList = parseJson(out.trim())
+ assert configList instanceof List
+
+ boolean originalDisableAutoCompaction = false
+ String originalBaseCompactionDupKeyMaxFileSizeMbytes = "1024"
+ for (Object ele in (List) configList) {
+ assert ele instanceof List<String>
+ if (((List<String>) ele)[0] == "disable_auto_compaction") {
+ originalDisableAutoCompaction =
Boolean.parseBoolean(((List<String>) ele)[2])
}
+ if (((List<String>) ele)[0] ==
"base_compaction_dup_key_max_file_size_mbytes") {
+ originalBaseCompactionDupKeyMaxFileSizeMbytes = ((List<String>)
ele)[2]
+ }
+ }
+
+ try {
+ // This test expects manual base compaction to be rejected with E-808
after building a
+ // single large base rowset. Background auto compaction can reshape
those rowsets, so keep
+ // it disabled cluster-wide while the case is running.
+ set_be_param("disable_auto_compaction", "true")
Review Comment:
This case now changes BE process-wide compaction settings on every backend,
but the suite is still tagged only as `p2`, so it will run in the normal
parallel suite pool. While it is between these calls and the `finally` restore,
any other concurrently running case that relies on automatic compaction or
duplicate-key base compaction can observe `disable_auto_compaction=true` or the
temporary 100 MB base-compaction limit and fail or skip compaction for reasons
unrelated to that case. The regression framework only routes suites containing
`nonConcurrent` to the single-thread executor, and nearby compaction tests that
mutate BE configs use that tag. Please mark this suite as non-concurrent as
well, e.g. keep the priority but change the group to `p2,nonConcurrent`.
--
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]
