github-actions[bot] commented on code in PR #66751:
URL: https://github.com/apache/doris/pull/66751#discussion_r3780789743
##########
fe/fe-common/src/main/java/org/apache/doris/common/Config.java:
##########
@@ -541,6 +550,40 @@ public class Config extends ConfigBase {
+ "a load job.")
public static short min_load_replica_num = -1;
+ @ConfField(mutable = true, masterOnly = true, description = "Minimum
number of successfully written replicas "
+ + "required in each availability zone for a load job.")
+ public static volatile String[] cross_az_succ_quorum = {};
+
+ public static Map<String, Integer> getCrossAzSuccQuorum() {
+ String[] config = cross_az_succ_quorum;
+ if (config == cachedCrossAzSuccQuorumConfig) {
+ return cachedCrossAzSuccQuorum;
+ }
+ synchronized (Config.class) {
+ config = cross_az_succ_quorum;
+ if (config == cachedCrossAzSuccQuorumConfig) {
+ return cachedCrossAzSuccQuorum;
+ }
+ Map<String, Integer> parsedConfig = new HashMap<>();
+ for (String item : config) {
+ String[] parts = item.split(":", -1);
+ try {
+ int configuredMin = Integer.parseInt(parts.length == 2 ?
parts[1].trim() : "");
+ if (parts[0].trim().isEmpty() || configuredMin < 0) {
+ throw new NumberFormatException();
+ }
+ parsedConfig.put(parts[0].trim(), configuredMin);
+ } catch (NumberFormatException e) {
Review Comment:
Reject the whole value before publishing it. Startup and `ADMIN SET FRONTEND
CONFIG` both use the generic `String[]` split-and-assign path, so
`az1:2,az2:not-a-number` is accepted; this catch then drops `az2` after the
update reported success. The ordinary two-`az1` quorum can consequently commit
with no `az2` enforcement, and the cached partial map suppresses later
warnings. Validate the grammar atomically in both startup and mutable-update
paths, including duplicate AZ keys, and change this test to expect a failed
configuration rather than a weakened policy.
##########
fe/fe-core/src/main/java/org/apache/doris/transaction/DatabaseTransactionMgr.java:
##########
@@ -627,6 +634,12 @@ private void checkCommitStatus(List<Table> tableList,
TransactionState transacti
throw new
TransactionCommitFailedException("could not find replica for tablet ["
+ tabletId + "], backend [" +
tabletBackend + "]");
}
+ if (!crossAzSuccQuorum.isEmpty()) {
+
backendLocationTags.computeIfAbsent(tabletBackend, backendId -> {
+ Backend backend =
env.getCurrentSystemInfo().getBackend(backendId);
+ return backend == null ? "" :
backend.getLocationTag().value;
Review Comment:
Consume a safely published backend-location snapshot here. `MODIFY BACKEND`
calls `setTagMap` on an already-published `Backend`, but `locationTag` is
non-volatile and unsynchronized, and that update does not republish the
volatile backend map; this commit thread shares neither the writer's monitor
nor another happens-before edge. After an `az1` backend is relabeled to `az2`,
a stale `az1` read can count one success in each zone although both current
successes are in `az2`. Publish the location through an immutable/volatile
snapshot or shared synchronization, and test concurrent relabel/commit behavior
around the defined snapshot boundary.
##########
fe/fe-core/src/main/java/org/apache/doris/transaction/DatabaseTransactionMgr.java:
##########
@@ -670,6 +683,37 @@ private void checkCommitStatus(List<Table> tableList,
TransactionState transacti
throw new
TabletQuorumFailedException(transactionId, errMsg);
}
+
+ for (Entry<String, Integer> entry :
crossAzSuccQuorum.entrySet()) {
Review Comment:
Make the writer's success criterion imply this new AZ floor. With allocation
`az1:2,az2:1` and policy `az2:1`, current v1/v2 writers can reach ordinary
quorum on the two `az1` replicas, end their bounded post-quorum wait, and emit
only those completed commit infos; a healthy slower `az2` replica is then
absent permanently. This loop rejects the load even when that replica could
finish within the load timeout, and an FE commit retry reuses the same finite
list. Initial and auto-partition payloads carry neither this policy nor
location tags, so current and older BEs cannot honor it. Derive and send a
compatible scalar criterion in both planning paths, or add a versioned writer
policy plus a rollout capability fence, and cover v1/v2 with a
delayed-remote-replica end-to-end 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]
