From b64d875b0f3bcc8d9c586b9a656666e1eb94a07d Mon Sep 17 00:00:00 2001 From: iosdev747 Date: Thu, 1 Aug 2024 21:30:04 +0530 Subject: [PATCH 1/8] propose changes for ns isolation policy --- pip/pip-369.md | 139 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100644 pip/pip-369.md diff --git a/pip/pip-369.md b/pip/pip-369.md new file mode 100644 index 0000000000000..f889f717d4113 --- /dev/null +++ b/pip/pip-369.md @@ -0,0 +1,139 @@ +# PIP-369: Flag based selective unload on changing ns-isolation-policy + +# Background knowledge + +In Apache Pulsar, namespace isolation policies are used to limit the ownership of certain subsets of namespaces to specific broker groups. +These policies are defined using regular expressions to match namespaces and specify primary and secondary broker groups along with failover policy configurations. +This ensures that the ownership of the specified namespaces is restricted to the designated broker groups. + +For more information, refer to the [Pulsar documentation on namespace isolation](https://pulsar.apache.org/docs/next/administration-isolation/#isolation-levels). + +# Motivation + +In Apache Pulsar 3.x, changing a namespace isolation policy results in unloading all namespace bundles that match the namespace's regular expression provided in the isolation policy. +This can be problematic for cases where the regex matches a large subset of namespaces, such as `tenant-x/.*`. One of such case is mentioned on this issue: [\[Bug\] Breaking contract b/w 2.9 / 2.10 and 3.0 for ns-isolation-policy](https://github.com/apache/pulsar/issues/23092). +This PIP aims to address the need to either prevent unnecessary unloading or provide a more granular approach to determine what should be unloaded. + +Some of the cases covered by this PIP are discussed in [#23092](https://github.com/apache/pulsar/issues/23092) by @grssam. +> - unload nothing as part of the set policy call +> - unload every matching namespace as part of the policy set call +> - unload only the changed namespaces (newly added + removed) + +# Goals + +## In Scope +This PIP proposes a flag-based approach to control what should be unloaded when an isolation policy is applied. The possible values for this flag are: +- **ALL**: Unload all namespaces that are either added or removed after updating the isolation policy. +- **CHANGED**: Only unload namespaces that are either added or removed due to the policy change. +- **NONE**: Do not unload anything. Unloading can occur naturally due to load balancing or can be done manually using the unload admin call. + +This flag will be present in both REST APIs and in admin client as an optional parameter. The default behavior will remain to unload all namespaces matching the new policy. + +## Out of Scope + +Applying concurrency reducer to limit how many async calls will happen in parallel is out of the scope for this PIP. +This should be addressed in a separate PIP, as solving the issue of infinite asynchronous calls probably requires changes to broker configurations and is a problem present in multiple areas. +# Detailed Design + +## Design & Implementation Details + +A new flag will be introduced to control the unloading behavior. This flag will be used by REST APIs and the Pulsar admin client. + +```java +enum UnloadOption { + ALL, // unloads everything, OLD ⋃ NEW + CHANGED, // unload namespaces delta, (new ⋃ old) - (new ∩ old) + NONE, // skip unloading anything, ϕ +}; +UnloadOption DEFAULT_VALUE = UnloadOption.ALL; +``` +Filters will be added based on the above when namespaces are selected for unload in set policy call. + +> **_NOTE:_** +> For 3.0 unchanged behaviour, the `ALL` option should only unload namespaces matching new policy (NEW). This matches the current behavior and maintains backward compatibility from implementation POV. +> +> For 4.x, +> 1. The behaviour for the `ALL` flag should change to unload everything matching either the old or new policy (union of both). +> 2. The default flag value should be `CHANGED`, so accidentally missing this flag while applying the policy shouldn't impact workloads already on the correct broker group. + +## Public-facing Changes + +New methods will be added to take this flag as an input. + +Class: pulsar/client/admin/Clusters.java +```java +void createNamespaceIsolationPolicy( + String cluster, String policyName, NamespaceIsolationData namespaceIsolationData, UnloadOption flag) throws PulsarAdminException; + +CompletableFuture createNamespaceIsolationPolicyAsync( + String cluster, String policyName, NamespaceIsolationData namespaceIsolationData, UnloadOption flag); + +void updateNamespaceIsolationPolicy( + String cluster, String policyName, NamespaceIsolationData namespaceIsolationData, UnloadOption flag) throws PulsarAdminException; + +CompletableFuture updateNamespaceIsolationPolicyAsync( + String cluster, String policyName, NamespaceIsolationData namespaceIsolationData, UnloadOption flag); +``` + +To maintain backward compatibility, default methods will be added. + +Class: pulsar/client/admin/Clusters.java +```java +default void createNamespaceIsolationPolicy( + String cluster, String policyName, NamespaceIsolationData namespaceIsolationData) throws PulsarAdminException { + createNamespaceIsolationPolicy(cluster, policyName, namespaceIsolationData, DEFAULT_VALUE); +} + +default CompletableFuture createNamespaceIsolationPolicyAsync( + String cluster, String policyName, NamespaceIsolationData namespaceIsolationData) { + return createNamespaceIsolationPolicyAsync(cluster, policyName, namespaceIsolationData, DEFAULT_VALUE); +} + +default void updateNamespaceIsolationPolicy( + String cluster, String policyName, NamespaceIsolationData namespaceIsolationData)throws PulsarAdminException { + updateNamespaceIsolationPolicy(cluster, policyName, namespaceIsolationData, DEFAULT_VALUE); +} + +default CompletableFuture updateNamespaceIsolationPolicyAsync( + String cluster, String policyName, NamespaceIsolationData namespaceIsolationData) { + return updateNamespaceIsolationPolicyAsync(cluster, policyName, namespaceIsolationData, DEFAULT_VALUE); +} +``` + +### Public API + +There is only a single change in setNamespaceIsolationPolicy() POST call. Addition of query param with the same defaults as discussed above. API params will remain unchanged. +Path: `/{cluster}/namespaceIsolationPolicies/{policyName}` +```java +@DefaultValue("UnloadOption.ALL") +@QueryParam("unloadBundles") UnloadOption flag +``` + +### CLI + +```shell +# set call will have an optional flag. Sample command as shown below: +# +pulsar-admin ns-isolation-policy set cluster-name policy-name --unload-option NONE +# Possible values for UnloadOption: [ALL, CHANGED, NONE] +``` + + +# Backward & Forward Compatibility + +This change does not affect how isolation policies work. The changes focus on the implementation of what namespaces will be unloaded based on policy changes. + +# Alternatives + +Boolean flag to decide either unload the delta namespaces (removed and added) without affecting unchanged namespaces or unload nothing. PR: https://github.com/apache/pulsar/pull/23094 + +Limitation: This approach does not consider cases where unloading is needed for every matching namespace as part of the policy set call. +Manual unloading would be required for unchanged namespaces not on the correct broker group. + +# Links + + +* Mailing List discussion thread: +* Mailing List voting thread: From bb7e2b9974812574543895c719a9c120a8f23fc8 Mon Sep 17 00:00:00 2001 From: iosdev747 Date: Thu, 1 Aug 2024 23:46:42 +0530 Subject: [PATCH 2/8] address review comments --- pip/pip-369.md | 98 +++++++++++++++++++++----------------------------- 1 file changed, 40 insertions(+), 58 deletions(-) diff --git a/pip/pip-369.md b/pip/pip-369.md index f889f717d4113..fd81020b25b1a 100644 --- a/pip/pip-369.md +++ b/pip/pip-369.md @@ -8,13 +8,27 @@ This ensures that the ownership of the specified namespaces is restricted to the For more information, refer to the [Pulsar documentation on namespace isolation](https://pulsar.apache.org/docs/next/administration-isolation/#isolation-levels). +# History/Context +In Apache Pulsar 2.7.1+, there was a flag introduced (`enableNamespaceIsolationUpdateOnTime`) that controlled whether to unload namespaces or not when a namespace isolation policy is applied. https://github.com/apache/pulsar/pull/8976 + +Later on, in 2.11, rework was done as part of [PIP-149](https://github.com/apache/pulsar/issues/14365) to make get/set isolationData calls async, +which resulted in namespaces to always get unloaded irrespective of `enableNamespaceIsolationUpdateOnTime` config, not adhering to this config at all. + +And now in 3.3, `enableNamespaceIsolationUpdateOnTime` broker config was deprecated as it no longer serves any purpose. https://github.com/apache/pulsar/pull/22449 + # Motivation In Apache Pulsar 3.x, changing a namespace isolation policy results in unloading all namespace bundles that match the namespace's regular expression provided in the isolation policy. -This can be problematic for cases where the regex matches a large subset of namespaces, such as `tenant-x/.*`. One of such case is mentioned on this issue: [\[Bug\] Breaking contract b/w 2.9 / 2.10 and 3.0 for ns-isolation-policy](https://github.com/apache/pulsar/issues/23092). +This can be problematic for cases where the regex matches a large subset of namespaces, such as `tenant-x/.*`. +One of such case is mentioned on this issue [#23092](https://github.com/apache/pulsar/issues/23092) where policy change resulted in 100+ namespace bundles to get unloaded. +And broker exhausted all the available connections due to too many unload calls happening at once resulting in 5xx response. +Other issues that happens with this approach are huge latency spikes as topics are unavailable until bundles are loaded back, increasing the pending produce calls. +The only benefit this approach serves is ensuring that all the namespaces matching the policy regex will come to correct broker group. +But when namespace bundles are already on the correct broker group (according to the policy), unloading those namespaces doesn't serve any purpose. + This PIP aims to address the need to either prevent unnecessary unloading or provide a more granular approach to determine what should be unloaded. -Some of the cases covered by this PIP are discussed in [#23092](https://github.com/apache/pulsar/issues/23092) by @grssam. +Some of the cases covered by this PIP are discussed in [#23094](https://github.com/apache/pulsar/issues/23094) by @grssam. > - unload nothing as part of the set policy call > - unload every matching namespace as part of the policy set call > - unload only the changed namespaces (newly added + removed) @@ -22,22 +36,24 @@ Some of the cases covered by this PIP are discussed in [#23092](https://github.c # Goals ## In Scope -This PIP proposes a flag-based approach to control what should be unloaded when an isolation policy is applied. The possible values for this flag are: +This PIP proposes a flag-based approach to control what should be unloaded when an isolation policy is applied. +The possible values for this flag are: - **ALL**: Unload all namespaces that are either added or removed after updating the isolation policy. - **CHANGED**: Only unload namespaces that are either added or removed due to the policy change. - **NONE**: Do not unload anything. Unloading can occur naturally due to load balancing or can be done manually using the unload admin call. -This flag will be present in both REST APIs and in admin client as an optional parameter. The default behavior will remain to unload all namespaces matching the new policy. +This flag will be a part of isolation policy data with defaults. Objective is to keep the default behavior unchanged on applying the new policy. ## Out of Scope Applying concurrency reducer to limit how many async calls will happen in parallel is out of the scope for this PIP. This should be addressed in a separate PIP, as solving the issue of infinite asynchronous calls probably requires changes to broker configurations and is a problem present in multiple areas. + # Detailed Design ## Design & Implementation Details -A new flag will be introduced to control the unloading behavior. This flag will be used by REST APIs and the Pulsar admin client. +A new flag will be introduced in `NamespaceIsolationData`. ```java enum UnloadOption { @@ -45,68 +61,35 @@ enum UnloadOption { CHANGED, // unload namespaces delta, (new ⋃ old) - (new ∩ old) NONE, // skip unloading anything, ϕ }; -UnloadOption DEFAULT_VALUE = UnloadOption.ALL; ``` Filters will be added based on the above when namespaces are selected for unload in set policy call. +`UnloadOption.ALL` will be the default in current version. > **_NOTE:_** -> For 3.0 unchanged behaviour, the `ALL` option should only unload namespaces matching new policy (NEW). This matches the current behavior and maintains backward compatibility from implementation POV. +> For 3.x unchanged behaviour, the `ALL` option should only unload namespaces matching new policy (NEW). This matches the current behavior and maintains backward compatibility from implementation POV. > > For 4.x, > 1. The behaviour for the `ALL` flag should change to unload everything matching either the old or new policy (union of both). > 2. The default flag value should be `CHANGED`, so accidentally missing this flag while applying the policy shouldn't impact workloads already on the correct broker group. -## Public-facing Changes - -New methods will be added to take this flag as an input. - -Class: pulsar/client/admin/Clusters.java -```java -void createNamespaceIsolationPolicy( - String cluster, String policyName, NamespaceIsolationData namespaceIsolationData, UnloadOption flag) throws PulsarAdminException; - -CompletableFuture createNamespaceIsolationPolicyAsync( - String cluster, String policyName, NamespaceIsolationData namespaceIsolationData, UnloadOption flag); - -void updateNamespaceIsolationPolicy( - String cluster, String policyName, NamespaceIsolationData namespaceIsolationData, UnloadOption flag) throws PulsarAdminException; - -CompletableFuture updateNamespaceIsolationPolicyAsync( - String cluster, String policyName, NamespaceIsolationData namespaceIsolationData, UnloadOption flag); -``` - -To maintain backward compatibility, default methods will be added. - -Class: pulsar/client/admin/Clusters.java -```java -default void createNamespaceIsolationPolicy( - String cluster, String policyName, NamespaceIsolationData namespaceIsolationData) throws PulsarAdminException { - createNamespaceIsolationPolicy(cluster, policyName, namespaceIsolationData, DEFAULT_VALUE); -} - -default CompletableFuture createNamespaceIsolationPolicyAsync( - String cluster, String policyName, NamespaceIsolationData namespaceIsolationData) { - return createNamespaceIsolationPolicyAsync(cluster, policyName, namespaceIsolationData, DEFAULT_VALUE); -} - -default void updateNamespaceIsolationPolicy( - String cluster, String policyName, NamespaceIsolationData namespaceIsolationData)throws PulsarAdminException { - updateNamespaceIsolationPolicy(cluster, policyName, namespaceIsolationData, DEFAULT_VALUE); -} - -default CompletableFuture updateNamespaceIsolationPolicyAsync( - String cluster, String policyName, NamespaceIsolationData namespaceIsolationData) { - return updateNamespaceIsolationPolicyAsync(cluster, policyName, namespaceIsolationData, DEFAULT_VALUE); -} -``` - ### Public API -There is only a single change in setNamespaceIsolationPolicy() POST call. Addition of query param with the same defaults as discussed above. API params will remain unchanged. +A new flag will be added in the NamespaceIsolationData. This changes the request body when set policy API is called. +To keep things backwards compatible, `unload_option` will be optional. API params will remain unchanged. + Path: `/{cluster}/namespaceIsolationPolicies/{policyName}` -```java -@DefaultValue("UnloadOption.ALL") -@QueryParam("unloadBundles") UnloadOption flag +```json +{ + "policy-name": { + "namespaces": [...], + "primary": [...], + "secondary": [...], + "auto_failover_policy": { + ... + }, + "unload_option": "ALL|CHANGED|NONE" + } +} ``` ### CLI @@ -118,14 +101,13 @@ pulsar-admin ns-isolation-policy set cluster-name policy-name --unload-option NO # Possible values for UnloadOption: [ALL, CHANGED, NONE] ``` - # Backward & Forward Compatibility -This change does not affect how isolation policies work. The changes focus on the implementation of what namespaces will be unloaded based on policy changes. +Added flag is optional, that doesn't require any changes to pre-existing policy data. If the flag is not present then default value shall be considered. # Alternatives -Boolean flag to decide either unload the delta namespaces (removed and added) without affecting unchanged namespaces or unload nothing. PR: https://github.com/apache/pulsar/pull/23094 +Boolean flag passed during set policy call to either unload the delta namespaces (removed and added) without affecting unchanged namespaces or unload nothing. PR: https://github.com/apache/pulsar/pull/23094 Limitation: This approach does not consider cases where unloading is needed for every matching namespace as part of the policy set call. Manual unloading would be required for unchanged namespaces not on the correct broker group. From d24cd0a318c135502cbfc252c6d2374ed40bd60d Mon Sep 17 00:00:00 2001 From: iosdev747 Date: Fri, 2 Aug 2024 18:24:47 +0530 Subject: [PATCH 3/8] typo fix --- pip/pip-369.md | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/pip/pip-369.md b/pip/pip-369.md index fd81020b25b1a..4ea1ec59abe50 100644 --- a/pip/pip-369.md +++ b/pip/pip-369.md @@ -38,9 +38,9 @@ Some of the cases covered by this PIP are discussed in [#23094](https://github.c ## In Scope This PIP proposes a flag-based approach to control what should be unloaded when an isolation policy is applied. The possible values for this flag are: -- **ALL**: Unload all namespaces that are either added or removed after updating the isolation policy. -- **CHANGED**: Only unload namespaces that are either added or removed due to the policy change. -- **NONE**: Do not unload anything. Unloading can occur naturally due to load balancing or can be done manually using the unload admin call. +- **all**: Unload all the namespaces that matches either old or new policy change. +- **changed**: Only unload namespaces that are either added or removed due to the policy change. +- **none**: Do not unload anything. Unloading can occur naturally due to load balancing or can be done manually using the unload admin call. This flag will be a part of isolation policy data with defaults. Objective is to keep the default behavior unchanged on applying the new policy. @@ -57,20 +57,20 @@ A new flag will be introduced in `NamespaceIsolationData`. ```java enum UnloadOption { - ALL, // unloads everything, OLD ⋃ NEW - CHANGED, // unload namespaces delta, (new ⋃ old) - (new ∩ old) - NONE, // skip unloading anything, ϕ + all, // unloads everything, OLD ⋃ NEW + changed, // unload namespaces delta, (new ⋃ old) - (new ∩ old) + none, // skip unloading anything, ϕ }; ``` Filters will be added based on the above when namespaces are selected for unload in set policy call. -`UnloadOption.ALL` will be the default in current version. +`UnloadOption.all` will be the default in current version. > **_NOTE:_** -> For 3.x unchanged behaviour, the `ALL` option should only unload namespaces matching new policy (NEW). This matches the current behavior and maintains backward compatibility from implementation POV. +> For 3.x unchanged behaviour, the `all` option should only unload namespaces matching new policy (NEW). This matches the current behavior and maintains backward compatibility from implementation POV. > > For 4.x, -> 1. The behaviour for the `ALL` flag should change to unload everything matching either the old or new policy (union of both). -> 2. The default flag value should be `CHANGED`, so accidentally missing this flag while applying the policy shouldn't impact workloads already on the correct broker group. +> 1. The behaviour for the `all` flag should change to unload everything matching either the old or new policy (union of both). +> 2. The default flag value should be `changed`, so accidentally missing this flag while applying the policy shouldn't impact workloads already on the correct broker group. ### Public API @@ -87,7 +87,7 @@ Path: `/{cluster}/namespaceIsolationPolicies/{policyName}` "auto_failover_policy": { ... }, - "unload_option": "ALL|CHANGED|NONE" + "unload_option": "all|changed|none" } } ``` @@ -97,8 +97,8 @@ Path: `/{cluster}/namespaceIsolationPolicies/{policyName}` ```shell # set call will have an optional flag. Sample command as shown below: # -pulsar-admin ns-isolation-policy set cluster-name policy-name --unload-option NONE -# Possible values for UnloadOption: [ALL, CHANGED, NONE] +pulsar-admin ns-isolation-policy set cluster-name policy-name --unload-option none +# Possible values for UnloadOption: [all, changed, none] ``` # Backward & Forward Compatibility From 3852857742f21c0307be9a13b9ab5bff0b62c767 Mon Sep 17 00:00:00 2001 From: iosdev747 Date: Fri, 2 Aug 2024 19:58:08 +0530 Subject: [PATCH 4/8] rename var unload_option to unload --- pip/pip-369.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pip/pip-369.md b/pip/pip-369.md index 4ea1ec59abe50..4e7da3e6afce7 100644 --- a/pip/pip-369.md +++ b/pip/pip-369.md @@ -56,17 +56,17 @@ This should be addressed in a separate PIP, as solving the issue of infinite asy A new flag will be introduced in `NamespaceIsolationData`. ```java -enum UnloadOption { +enum unload { all, // unloads everything, OLD ⋃ NEW changed, // unload namespaces delta, (new ⋃ old) - (new ∩ old) none, // skip unloading anything, ϕ }; ``` Filters will be added based on the above when namespaces are selected for unload in set policy call. -`UnloadOption.all` will be the default in current version. +`unload.all` will be the default in current version. > **_NOTE:_** -> For 3.x unchanged behaviour, the `all` option should only unload namespaces matching new policy (NEW). This matches the current behavior and maintains backward compatibility from implementation POV. +> For 3.x unchanged behaviour, the `all` unload option should only unload namespaces matching new policy (NEW). This matches the current behavior and maintains backward compatibility from implementation POV. > > For 4.x, > 1. The behaviour for the `all` flag should change to unload everything matching either the old or new policy (union of both). @@ -75,7 +75,7 @@ Filters will be added based on the above when namespaces are selected for unload ### Public API A new flag will be added in the NamespaceIsolationData. This changes the request body when set policy API is called. -To keep things backwards compatible, `unload_option` will be optional. API params will remain unchanged. +To keep things backwards compatible, `unload` will be optional. API params will remain unchanged. Path: `/{cluster}/namespaceIsolationPolicies/{policyName}` ```json @@ -87,7 +87,7 @@ Path: `/{cluster}/namespaceIsolationPolicies/{policyName}` "auto_failover_policy": { ... }, - "unload_option": "all|changed|none" + "unload": "all|changed|none" } } ``` @@ -98,7 +98,7 @@ Path: `/{cluster}/namespaceIsolationPolicies/{policyName}` # set call will have an optional flag. Sample command as shown below: # pulsar-admin ns-isolation-policy set cluster-name policy-name --unload-option none -# Possible values for UnloadOption: [all, changed, none] +# Possible values for unload: [all, changed, none] ``` # Backward & Forward Compatibility From 5c4d18075c4e3e292baddc47949d397795ab19f6 Mon Sep 17 00:00:00 2001 From: iosdev747 Date: Fri, 2 Aug 2024 21:32:10 +0530 Subject: [PATCH 5/8] update discussion link --- pip/pip-369.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pip/pip-369.md b/pip/pip-369.md index 4e7da3e6afce7..e9ce82bf30910 100644 --- a/pip/pip-369.md +++ b/pip/pip-369.md @@ -117,5 +117,5 @@ Manual unloading would be required for unchanged namespaces not on the correct b -* Mailing List discussion thread: +* Mailing List discussion thread: https://lists.apache.org/thread/6f8k1typ48817w65pjh6orhks1smpbqg * Mailing List voting thread: From 72aedac4e7de85f2658cbd1e7b4f9625ceb3b1c6 Mon Sep 17 00:00:00 2001 From: iosdev747 Date: Fri, 9 Aug 2024 18:51:46 +0530 Subject: [PATCH 6/8] minor refactor --- pip/pip-369.md | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/pip/pip-369.md b/pip/pip-369.md index e9ce82bf30910..23f0d7667de4e 100644 --- a/pip/pip-369.md +++ b/pip/pip-369.md @@ -38,7 +38,7 @@ Some of the cases covered by this PIP are discussed in [#23094](https://github.c ## In Scope This PIP proposes a flag-based approach to control what should be unloaded when an isolation policy is applied. The possible values for this flag are: -- **all**: Unload all the namespaces that matches either old or new policy change. +- **all_matching**: Unload all the namespaces that matches either old or new policy change. - **changed**: Only unload namespaces that are either added or removed due to the policy change. - **none**: Do not unload anything. Unloading can occur naturally due to load balancing or can be done manually using the unload admin call. @@ -56,26 +56,26 @@ This should be addressed in a separate PIP, as solving the issue of infinite asy A new flag will be introduced in `NamespaceIsolationData`. ```java -enum unload { - all, // unloads everything, OLD ⋃ NEW +enum unload_scope { + all_matching, // unloads everything, OLD ⋃ NEW changed, // unload namespaces delta, (new ⋃ old) - (new ∩ old) none, // skip unloading anything, ϕ }; ``` Filters will be added based on the above when namespaces are selected for unload in set policy call. -`unload.all` will be the default in current version. +`unload_scope.all_matching` will be the default in current version. > **_NOTE:_** -> For 3.x unchanged behaviour, the `all` unload option should only unload namespaces matching new policy (NEW). This matches the current behavior and maintains backward compatibility from implementation POV. +> For 3.x unchanged behaviour, the `all_matching` unload_scope option should only unload namespaces matching new policy (NEW). This matches the current behavior and maintains backward compatibility from implementation POV. > > For 4.x, -> 1. The behaviour for the `all` flag should change to unload everything matching either the old or new policy (union of both). +> 1. The behaviour for the `all_matching` flag should change to unload everything matching either the old or new policy (union of both). > 2. The default flag value should be `changed`, so accidentally missing this flag while applying the policy shouldn't impact workloads already on the correct broker group. ### Public API A new flag will be added in the NamespaceIsolationData. This changes the request body when set policy API is called. -To keep things backwards compatible, `unload` will be optional. API params will remain unchanged. +To keep things backwards compatible, `unload_scope` will be optional. API params will remain unchanged. Path: `/{cluster}/namespaceIsolationPolicies/{policyName}` ```json @@ -87,7 +87,7 @@ Path: `/{cluster}/namespaceIsolationPolicies/{policyName}` "auto_failover_policy": { ... }, - "unload": "all|changed|none" + "unload_scope": "all_matching|changed|none" } } ``` @@ -97,8 +97,8 @@ Path: `/{cluster}/namespaceIsolationPolicies/{policyName}` ```shell # set call will have an optional flag. Sample command as shown below: # -pulsar-admin ns-isolation-policy set cluster-name policy-name --unload-option none -# Possible values for unload: [all, changed, none] +pulsar-admin ns-isolation-policy set cluster-name policy-name --unload-scope none +# Possible values for unload-scope: [all_matching, changed, none] ``` # Backward & Forward Compatibility @@ -119,3 +119,6 @@ Updated afterwards --> * Mailing List discussion thread: https://lists.apache.org/thread/6f8k1typ48817w65pjh6orhks1smpbqg * Mailing List voting thread: + + +PS: This PIP should get cherry-picked to 3.0.x as it provides a way to resolve the bug mentioned at [#23092](https://github.com/apache/pulsar/issues/23092) which exist in all the production system today. \ No newline at end of file From 116522909d196daa3f89b96f6a96f3e2fdce64b7 Mon Sep 17 00:00:00 2001 From: iosdev747 Date: Fri, 9 Aug 2024 22:59:59 +0530 Subject: [PATCH 7/8] add discussion thread --- pip/pip-369.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pip/pip-369.md b/pip/pip-369.md index 23f0d7667de4e..840d06240dc25 100644 --- a/pip/pip-369.md +++ b/pip/pip-369.md @@ -118,7 +118,7 @@ Manual unloading would be required for unchanged namespaces not on the correct b Updated afterwards --> * Mailing List discussion thread: https://lists.apache.org/thread/6f8k1typ48817w65pjh6orhks1smpbqg -* Mailing List voting thread: +* Mailing List voting thread: https://lists.apache.org/thread/0pj3llwpcy73mrs5s3l5t8kctn2mzyf7 PS: This PIP should get cherry-picked to 3.0.x as it provides a way to resolve the bug mentioned at [#23092](https://github.com/apache/pulsar/issues/23092) which exist in all the production system today. \ No newline at end of file From e36e98c7005930488e9ea8ba6251d564130346ab Mon Sep 17 00:00:00 2001 From: iosdev747 Date: Sat, 10 Aug 2024 04:50:59 +0530 Subject: [PATCH 8/8] minor refactor --- pip/pip-369.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pip/pip-369.md b/pip/pip-369.md index 840d06240dc25..9aeb598110d72 100644 --- a/pip/pip-369.md +++ b/pip/pip-369.md @@ -56,17 +56,17 @@ This should be addressed in a separate PIP, as solving the issue of infinite asy A new flag will be introduced in `NamespaceIsolationData`. ```java -enum unload_scope { +enum UnloadScope { all_matching, // unloads everything, OLD ⋃ NEW changed, // unload namespaces delta, (new ⋃ old) - (new ∩ old) none, // skip unloading anything, ϕ }; ``` Filters will be added based on the above when namespaces are selected for unload in set policy call. -`unload_scope.all_matching` will be the default in current version. +`UnloadScope.all_matching` will be the default in current version. > **_NOTE:_** -> For 3.x unchanged behaviour, the `all_matching` unload_scope option should only unload namespaces matching new policy (NEW). This matches the current behavior and maintains backward compatibility from implementation POV. +> For 3.x unchanged behaviour, the `all_matching` UnloadScope option should only unload namespaces matching new policy (NEW). This matches the current behavior and maintains backward compatibility from implementation POV. > > For 4.x, > 1. The behaviour for the `all_matching` flag should change to unload everything matching either the old or new policy (union of both).