eolivelli commented on a change in pull request #11330:
URL: https://github.com/apache/pulsar/pull/11330#discussion_r670523160



##########
File path: 
pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/ResourceQuota.java
##########
@@ -18,11 +18,14 @@
  */
 package org.apache.pulsar.common.policies.data;
 
-import java.util.Objects;
+import lombok.Data;
+import lombok.experimental.Accessors;
 
 /**
  * Resource quota for a namespace or namespace bundle.
  */
+@Data
+@Accessors(chain = true)

Review comment:
       with `chain = true` we are introducing a breaking API change (changing 
the return type for the setters).
   can we avoid to introduce this breaking change ?

##########
File path: 
pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/ResourceQuota.java
##########
@@ -47,132 +50,6 @@ public ResourceQuota() {
         dynamic = true;
     }
 
-    /**
-     * Set incoming message rate quota.
-     *
-     * @param msgRateIn
-     *            incoming messages rate quota (msg/sec)
-     */
-    public void setMsgRateIn(double msgRateIn) {
-        this.msgRateIn = msgRateIn;
-    }
-
-    /**
-     * Get incoming message rate quota.
-     *
-     * @return incoming message rate quota (msg/sec)
-     */
-    public double getMsgRateIn() {
-        return this.msgRateIn;
-    }
-
-    /**
-     * Set outgoing message rate quota.
-     *
-     * @param msgRateOut
-     *            outgoing messages rate quota (msg/sec)
-     */
-    public void setMsgRateOut(double msgRateOut) {
-        this.msgRateOut = msgRateOut;
-    }
-
-    /**
-     * Get outgoing message rate quota.
-     *
-     * @return outgoing message rate quota (msg/sec)
-     */
-    public double getMsgRateOut() {
-        return this.msgRateOut;
-    }
-
-    /**
-     * Set inbound bandwidth quota.
-     *
-     * @param bandwidthIn
-     *            inbound bandwidth quota (bytes/sec)
-     */
-    public void setBandwidthIn(double bandwidthIn) {
-        this.bandwidthIn = bandwidthIn;
-    }
-
-    /**
-     * Get inbound bandwidth quota.
-     *
-     * @return inbound bandwidth quota (bytes/sec)
-     */
-    public double getBandwidthIn() {
-        return this.bandwidthIn;
-    }
-
-    /**
-     * Set outbound bandwidth quota.
-     *
-     * @param bandwidthOut
-     *            outbound bandwidth quota (bytes/sec)
-     */
-    public void setBandwidthOut(double bandwidthOut) {
-        this.bandwidthOut = bandwidthOut;
-    }
-
-    /**
-     * Get outbound bandwidth quota.
-     *
-     * @return outbound bandwidth quota (bytes/sec)
-     */
-    public double getBandwidthOut() {
-        return this.bandwidthOut;
-    }
-
-    /**
-     * Set memory quota.
-     *
-     * @param memory
-     *            memory quota (Mbytes)
-     */
-    public void setMemory(double memory) {
-        this.memory = memory;
-    }
-
-    /**
-     * Get memory quota.
-     *
-     * @return memory quota (Mbytes)
-     */
-    public double getMemory() {
-        return this.memory;
-    }
-
-    /**
-     * Set dynamic to true/false.
-     *
-     * @param dynamic
-     *            allow the quota to be dynamically re-calculated
-     */
-    public void setDynamic(boolean dynamic) {
-        this.dynamic = dynamic;
-    }
-
-    /**
-     * Get dynamic setting.
-     *
-     * @return is dynamic or not
-     */
-    public boolean getDynamic() {

Review comment:
       also we are changing from `getDynamic` to `isDynamic`

##########
File path: 
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/ResourceQuotasBase.java
##########
@@ -87,13 +84,9 @@ protected void 
internalSetNamespaceBundleResourceQuota(String bundleRange, Resou
         NamespaceBundle nsBundle = validateNamespaceBundleRange(namespaceName, 
policies.bundles, bundleRange);
 
         try {
-            
pulsar().getLocalZkCacheService().getResourceQuotaCache().setQuota(nsBundle, 
quota);
+            
pulsar().getBrokerService().getBundlesQuotas().setResourceQuota(nsBundle, 
quota).join();
             log.info("[{}] Successfully set resource quota for namespace 
bundle {}", clientAppId(),
                     nsBundle.toString());
-        } catch (KeeperException.NoNodeException e) {
-            log.warn("[{}] Failed to set resource quota for namespace bundle 
{}: concurrent modification",
-                    clientAppId(), nsBundle.toString());
-            throw new RestException(Status.CONFLICT, "Concurrent modification 
on namespace bundle quota");

Review comment:
       are we losing this CONFLICT case ?
   I believe it is not a big deal, so this change is good.
   But I would like to see your opinion @merlimat 




-- 
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: commits-unsubscr...@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to