codelipenghui commented on code in PR #25304:
URL: https://github.com/apache/pulsar/pull/25304#discussion_r2920544531
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/namespace/NamespaceService.java:
##########
@@ -142,12 +142,10 @@ public class NamespaceService implements AutoCloseable {
public static final int BUNDLE_SPLIT_RETRY_LIMIT = 7;
public static final String SLA_NAMESPACE_PROPERTY = "sla-monitor";
- public static final Pattern HEARTBEAT_NAMESPACE_PATTERN =
Pattern.compile("pulsar/[^/]+/([^:]+:\\d+)");
- public static final Pattern HEARTBEAT_NAMESPACE_PATTERN_V2 =
Pattern.compile("pulsar/([^:]+:\\d+)");
- public static final Pattern SLA_NAMESPACE_PATTERN =
Pattern.compile(SLA_NAMESPACE_PROPERTY + "/[^/]+/([^:]+:\\d+)");
- public static final String HEARTBEAT_NAMESPACE_FMT = "pulsar/%s/%s";
- public static final String HEARTBEAT_NAMESPACE_FMT_V2 = "pulsar/%s";
- public static final String SLA_NAMESPACE_FMT = SLA_NAMESPACE_PROPERTY +
"/%s/%s";
+ public static final Pattern HEARTBEAT_NAMESPACE_PATTERN =
Pattern.compile("pulsar/([^:]+:\\d+)");
+ public static final Pattern SLA_NAMESPACE_PATTERN =
Pattern.compile(SLA_NAMESPACE_PROPERTY + "/([^:]+:\\d+)");
+ public static final String HEARTBEAT_NAMESPACE_FMT = "pulsar/%s";
+ public static final String SLA_NAMESPACE_FMT = SLA_NAMESPACE_PROPERTY +
"/%s";
Review Comment:
**Rolling upgrade concern:** Heartbeat format changed from
`pulsar/cluster/brokerId` to `pulsar/brokerId` and SLA from
`sla-monitor/cluster/brokerId` to `sla-monitor/brokerId`. The V1 pattern
matchers (`HEARTBEAT_NAMESPACE_PATTERN` with `pulsar/[^/]+/...`) are removed.
During rolling upgrade, new brokers won't recognize old brokers' V1
heartbeat namespaces. Old V1 heartbeat/SLA namespace entries in metadata could
cause `IllegalArgumentException` when parsed as `NamespaceName` (3-part names
now rejected).
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -835,17 +805,7 @@ protected CompletableFuture<Void>
internalSetNamespaceReplicationClusters(List<S
if (CollectionUtils.isEmpty(clusterIds)) {
throw new RestException(Status.PRECONDITION_FAILED,
"ClusterIds should not be null or empty");
}
- if (!namespaceName.isGlobal() && !(clusterIds.size() == 1
- &&
clusterIds.get(0).equals(pulsar().getConfiguration().getClusterName()))) {
- throw new RestException(Status.PRECONDITION_FAILED,
- "Cannot set replication on a non-global
namespace");
- }
- Set<String> replicationClusterSet =
Sets.newHashSet(clusterIds);
- if (replicationClusterSet.contains("global")) {
- throw new RestException(Status.PRECONDITION_FAILED,
- "Cannot specify global in the list of
replication clusters");
- }
- return replicationClusterSet;
+ return Sets.newHashSet(clusterIds);
Review Comment:
**Note:** Removed validation that "global" cannot be in replication clusters
list. If old scripts or tooling pass "global" as a replication cluster, it
would now be accepted but likely fail downstream when trying to replicate to a
non-existent cluster.
--
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]
