[GitHub] activemq-artemis pull request #2191: ARTEMIS-1987 - Add consumer window size...

classic Classic list List threaded Threaded
24 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2191: ARTEMIS-1987 - Add consumer window size...

wy96f
GitHub user cshannon opened a pull request:

    https://github.com/apache/activemq-artemis/pull/2191

    ARTEMIS-1987 - Add consumer window size to AddressSettings

    Support configuring a default consumer window size via AddressSettings
    which will allow sensible defaults to be used by address type

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/cshannon/activemq-artemis ARTEMIS-1987

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/activemq-artemis/pull/2191.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #2191
   
----
commit 878b53e455e402ea0de0d64dcd53550f16273664
Author: Christopher L. Shannon (cshannon) <christopher.l.shannon@...>
Date:   2018-04-09T12:20:57Z

    ARTEMIS-1987 - Add consumer window size to AddressSettings
   
    Support configuring a default consumer window size via AddressSettings
    which will allow sensible defaults to be used by address type

----


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2191: ARTEMIS-1987 - Add consumer window size to Add...

wy96f
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2191
 
    See inline comments


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2191: ARTEMIS-1987 - Add consumer window size...

wy96f
In reply to this post by wy96f
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2191#discussion_r204784325
 
    --- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ServerLocatorImpl.java ---
    @@ -347,7 +347,7 @@ private ServerLocatorImpl(final Topology topology,
     
           minLargeMessageSize = ActiveMQClient.DEFAULT_MIN_LARGE_MESSAGE_SIZE;
     
    -      consumerWindowSize = ActiveMQClient.DEFAULT_CONSUMER_WINDOW_SIZE;
    +      consumerWindowSize = -1;
    --- End diff --
   
    Should still be defaulted to a constant


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2191: ARTEMIS-1987 - Add consumer window size...

wy96f
In reply to this post by wy96f
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2191#discussion_r204785897
 
    --- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ActiveMQSessionContext.java ---
    @@ -822,6 +833,20 @@ public void resetMetadata(HashMap<String, String> metaDataToSend) {
           }
        }
     
    +   @Override
    +   public int getDefaultConsumerWindowSize(SimpleString address) throws ActiveMQException {
    +      if (defaultConsumerWindowSize != null) {
    +         return defaultConsumerWindowSize;
    +      } else if (sessionChannel.supports(PacketImpl.SESS_CONS_WINDOW_SIZE_RESP, getServerVersion())) {
    +         Packet packet = sessionChannel.sendBlocking(new ConsumerWindowSizeQueryMessage(address), PacketImpl.SESS_CONS_WINDOW_SIZE_RESP);
    +         ConsumerWindowSizeQueryResponseMessage response = (ConsumerWindowSizeQueryResponseMessage) packet;
    --- End diff --
   
    Could this not be returned in the create consumer response or address settings lookup, to avoid extra calls. Imagine further defaults etc if everything was an individual request it would bloat fast.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2191: ARTEMIS-1987 - Add consumer window size to Add...

wy96f
In reply to this post by wy96f
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2191
 
    Usually the property on window-size is dependent on the semantic of the client.
   
    Like a client is processing big DB operations..so the client will have the information about it.
   
   
    Are you sure about this? It's a nice feature and work well done.. but I'm just wondering on the use-case where you would apply it in a useful manner.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2191: ARTEMIS-1987 - Add consumer window size...

wy96f
In reply to this post by wy96f
Github user cshannon commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2191#discussion_r204819904
 
    --- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ServerLocatorImpl.java ---
    @@ -347,7 +347,7 @@ private ServerLocatorImpl(final Topology topology,
     
           minLargeMessageSize = ActiveMQClient.DEFAULT_MIN_LARGE_MESSAGE_SIZE;
     
    -      consumerWindowSize = ActiveMQClient.DEFAULT_CONSUMER_WINDOW_SIZE;
    +      consumerWindowSize = -1;
    --- End diff --
   
    I can change it back but I was using -1 to figure out whether or not the value was set.  The issue is that if someone manually wants to set the client to the same value as that constant then it will always be overriden by the default from the server.  The idea is if the client sets the value that should be used instead....Not sure how to get around that without adding another flag to indicate the user overrode the value (or set it to null by default)


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2191: ARTEMIS-1987 - Add consumer window size...

wy96f
In reply to this post by wy96f
Github user cshannon commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2191#discussion_r204819939
 
    --- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ActiveMQSessionContext.java ---
    @@ -822,6 +833,20 @@ public void resetMetadata(HashMap<String, String> metaDataToSend) {
           }
        }
     
    +   @Override
    +   public int getDefaultConsumerWindowSize(SimpleString address) throws ActiveMQException {
    +      if (defaultConsumerWindowSize != null) {
    +         return defaultConsumerWindowSize;
    +      } else if (sessionChannel.supports(PacketImpl.SESS_CONS_WINDOW_SIZE_RESP, getServerVersion())) {
    +         Packet packet = sessionChannel.sendBlocking(new ConsumerWindowSizeQueryMessage(address), PacketImpl.SESS_CONS_WINDOW_SIZE_RESP);
    +         ConsumerWindowSizeQueryResponseMessage response = (ConsumerWindowSizeQueryResponseMessage) packet;
    --- End diff --
   
    @michaelandrepearce - yes I could probably just extend the SessionQueueQueryResponseMessage and create a SessionQueueQueryResponseMessage_V4 and add that info to it...I will go ahead and do that and rework this


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2191: ARTEMIS-1987 - Add consumer window size to Add...

wy96f
In reply to this post by wy96f
Github user cshannon commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2191
 
    @clebertsuconic - My use case is for the "service provider" case where the administrators want to protect a shared resource broker that services many different customers and fine tune settings.  In this case there is often many different clients competing for resources and an administrator may want to tune the broker to lower the window size for slow or bad clients, etc.  This doesn't prevent the user from overriding the setting on their client if they really want to change the window size of course.
   
    While it's probably not the most common use case it's the scenario I deal with the most and it is very useful coming from a 5.x broker where prefetch can be configured per policy.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2191: ARTEMIS-1987 - Add consumer window size to Add...

wy96f
In reply to this post by wy96f
Github user cshannon commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2191
 
    @clebertsuconic and @michaelandrepearce - PR has been updated


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2191: ARTEMIS-1987 - Add consumer window size to Add...

wy96f
In reply to this post by wy96f
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2191
 
    Is it worth a few other client settings to also be able to be defaulted centrally in broker?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2191: ARTEMIS-1987 - Add consumer window size...

wy96f
In reply to this post by wy96f
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2191#discussion_r204860714
 
    --- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/wireformat/SessionQueueQueryResponseMessage_V4.java ---
    @@ -0,0 +1,151 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements. See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License. You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.activemq.artemis.core.protocol.core.impl.wireformat;
    +
    +import org.apache.activemq.artemis.api.core.ActiveMQBuffer;
    +import org.apache.activemq.artemis.api.core.RoutingType;
    +import org.apache.activemq.artemis.api.core.SimpleString;
    +import org.apache.activemq.artemis.api.core.client.ActiveMQClient;
    +import org.apache.activemq.artemis.api.core.client.ClientSession;
    +import org.apache.activemq.artemis.core.client.impl.QueueQueryImpl;
    +import org.apache.activemq.artemis.core.server.QueueQueryResult;
    +
    +public class SessionQueueQueryResponseMessage_V4 extends SessionQueueQueryResponseMessage_V3 {
    +
    +   protected int defaultConsumerWindowSize;
    +
    +   public SessionQueueQueryResponseMessage_V4(final QueueQueryResult result) {
    +      this(result.getName(), result.getAddress(), result.isDurable(), result.isTemporary(), result.getFilterString(), result.getConsumerCount(), result.getMessageCount(), result.isExists(), result.isAutoCreateQueues(), result.isAutoCreated(), result.isPurgeOnNoConsumers(), result.getRoutingType(), result.getMaxConsumers(), result.isExclusive(), result.isLastValue(), result.getDefaultConsumerWindowSize());
    --- End diff --
   
    @cshannon just wondering.. wouldn't be worth to invest here to return a hashMap instead?
   
    We keep creating V4, V5... if we start using a Map, we wouldn't need to change the wire for this ever again.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2191: ARTEMIS-1987 - Add consumer window size...

wy96f
In reply to this post by wy96f
Github user cshannon commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2191#discussion_r204867142
 
    --- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/wireformat/SessionQueueQueryResponseMessage_V4.java ---
    @@ -0,0 +1,151 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements. See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License. You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.activemq.artemis.core.protocol.core.impl.wireformat;
    +
    +import org.apache.activemq.artemis.api.core.ActiveMQBuffer;
    +import org.apache.activemq.artemis.api.core.RoutingType;
    +import org.apache.activemq.artemis.api.core.SimpleString;
    +import org.apache.activemq.artemis.api.core.client.ActiveMQClient;
    +import org.apache.activemq.artemis.api.core.client.ClientSession;
    +import org.apache.activemq.artemis.core.client.impl.QueueQueryImpl;
    +import org.apache.activemq.artemis.core.server.QueueQueryResult;
    +
    +public class SessionQueueQueryResponseMessage_V4 extends SessionQueueQueryResponseMessage_V3 {
    +
    +   protected int defaultConsumerWindowSize;
    +
    +   public SessionQueueQueryResponseMessage_V4(final QueueQueryResult result) {
    +      this(result.getName(), result.getAddress(), result.isDurable(), result.isTemporary(), result.getFilterString(), result.getConsumerCount(), result.getMessageCount(), result.isExists(), result.isAutoCreateQueues(), result.isAutoCreated(), result.isPurgeOnNoConsumers(), result.getRoutingType(), result.getMaxConsumers(), result.isExclusive(), result.isLastValue(), result.getDefaultConsumerWindowSize());
    --- End diff --
   
    So for V4 just use a hash map for all values? And then have all the getters reference the map? That would be fine with me especially because @michaelandrepearce mentioned maybe adding other settings as well...while we could add some settings now there will always be more changes
   
    I can make the change tomorrow morning


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2191: ARTEMIS-1987 - Add consumer window size...

wy96f
In reply to this post by wy96f
Github user cshannon commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2191#discussion_r204867293
 
    --- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/wireformat/SessionQueueQueryResponseMessage_V4.java ---
    @@ -0,0 +1,151 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements. See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License. You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.activemq.artemis.core.protocol.core.impl.wireformat;
    +
    +import org.apache.activemq.artemis.api.core.ActiveMQBuffer;
    +import org.apache.activemq.artemis.api.core.RoutingType;
    +import org.apache.activemq.artemis.api.core.SimpleString;
    +import org.apache.activemq.artemis.api.core.client.ActiveMQClient;
    +import org.apache.activemq.artemis.api.core.client.ClientSession;
    +import org.apache.activemq.artemis.core.client.impl.QueueQueryImpl;
    +import org.apache.activemq.artemis.core.server.QueueQueryResult;
    +
    +public class SessionQueueQueryResponseMessage_V4 extends SessionQueueQueryResponseMessage_V3 {
    +
    +   protected int defaultConsumerWindowSize;
    +
    +   public SessionQueueQueryResponseMessage_V4(final QueueQueryResult result) {
    +      this(result.getName(), result.getAddress(), result.isDurable(), result.isTemporary(), result.getFilterString(), result.getConsumerCount(), result.getMessageCount(), result.isExists(), result.isAutoCreateQueues(), result.isAutoCreated(), result.isPurgeOnNoConsumers(), result.getRoutingType(), result.getMaxConsumers(), result.isExclusive(), result.isLastValue(), result.getDefaultConsumerWindowSize());
    --- End diff --
   
    Or actually do you just want to keep existing stuff the way it is and only use the Map for new values starting with defaultConsumerWindowSize?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2191: ARTEMIS-1987 - Add consumer window size...

wy96f
In reply to this post by wy96f
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2191#discussion_r204881984
 
    --- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/wireformat/SessionQueueQueryResponseMessage_V4.java ---
    @@ -0,0 +1,151 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements. See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License. You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.activemq.artemis.core.protocol.core.impl.wireformat;
    +
    +import org.apache.activemq.artemis.api.core.ActiveMQBuffer;
    +import org.apache.activemq.artemis.api.core.RoutingType;
    +import org.apache.activemq.artemis.api.core.SimpleString;
    +import org.apache.activemq.artemis.api.core.client.ActiveMQClient;
    +import org.apache.activemq.artemis.api.core.client.ClientSession;
    +import org.apache.activemq.artemis.core.client.impl.QueueQueryImpl;
    +import org.apache.activemq.artemis.core.server.QueueQueryResult;
    +
    +public class SessionQueueQueryResponseMessage_V4 extends SessionQueueQueryResponseMessage_V3 {
    +
    +   protected int defaultConsumerWindowSize;
    +
    +   public SessionQueueQueryResponseMessage_V4(final QueueQueryResult result) {
    +      this(result.getName(), result.getAddress(), result.isDurable(), result.isTemporary(), result.getFilterString(), result.getConsumerCount(), result.getMessageCount(), result.isExists(), result.isAutoCreateQueues(), result.isAutoCreated(), result.isPurgeOnNoConsumers(), result.getRoutingType(), result.getMaxConsumers(), result.isExclusive(), result.isLastValue(), result.getDefaultConsumerWindowSize());
    --- End diff --
   
    I would keep existing stuff the way it is... and use the map for new values.
   
    Anyway.. that's a suggestion... if you don't have time to do it now I will merge it as is... let me know?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2191: ARTEMIS-1987 - Add consumer window size...

wy96f
In reply to this post by wy96f
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2191#discussion_r204908501
 
    --- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/wireformat/SessionQueueQueryResponseMessage_V4.java ---
    @@ -0,0 +1,151 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements. See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License. You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.activemq.artemis.core.protocol.core.impl.wireformat;
    +
    +import org.apache.activemq.artemis.api.core.ActiveMQBuffer;
    +import org.apache.activemq.artemis.api.core.RoutingType;
    +import org.apache.activemq.artemis.api.core.SimpleString;
    +import org.apache.activemq.artemis.api.core.client.ActiveMQClient;
    +import org.apache.activemq.artemis.api.core.client.ClientSession;
    +import org.apache.activemq.artemis.core.client.impl.QueueQueryImpl;
    +import org.apache.activemq.artemis.core.server.QueueQueryResult;
    +
    +public class SessionQueueQueryResponseMessage_V4 extends SessionQueueQueryResponseMessage_V3 {
    +
    +   protected int defaultConsumerWindowSize;
    +
    +   public SessionQueueQueryResponseMessage_V4(final QueueQueryResult result) {
    +      this(result.getName(), result.getAddress(), result.isDurable(), result.isTemporary(), result.getFilterString(), result.getConsumerCount(), result.getMessageCount(), result.isExists(), result.isAutoCreateQueues(), result.isAutoCreated(), result.isPurgeOnNoConsumers(), result.getRoutingType(), result.getMaxConsumers(), result.isExclusive(), result.isLastValue(), result.getDefaultConsumerWindowSize());
    --- End diff --
   
    I would keep it as is. I think a dicussion is needes as currently having the version and typing means there is a strong contract, moving to a map removes the strong versioned contract.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2191: ARTEMIS-1987 - Add consumer window size...

wy96f
In reply to this post by wy96f
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2191#discussion_r204915141
 
    --- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/wireformat/SessionQueueQueryResponseMessage_V4.java ---
    @@ -0,0 +1,151 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements. See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License. You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.activemq.artemis.core.protocol.core.impl.wireformat;
    +
    +import org.apache.activemq.artemis.api.core.ActiveMQBuffer;
    +import org.apache.activemq.artemis.api.core.RoutingType;
    +import org.apache.activemq.artemis.api.core.SimpleString;
    +import org.apache.activemq.artemis.api.core.client.ActiveMQClient;
    +import org.apache.activemq.artemis.api.core.client.ClientSession;
    +import org.apache.activemq.artemis.core.client.impl.QueueQueryImpl;
    +import org.apache.activemq.artemis.core.server.QueueQueryResult;
    +
    +public class SessionQueueQueryResponseMessage_V4 extends SessionQueueQueryResponseMessage_V3 {
    +
    +   protected int defaultConsumerWindowSize;
    +
    +   public SessionQueueQueryResponseMessage_V4(final QueueQueryResult result) {
    +      this(result.getName(), result.getAddress(), result.isDurable(), result.isTemporary(), result.getFilterString(), result.getConsumerCount(), result.getMessageCount(), result.isExists(), result.isAutoCreateQueues(), result.isAutoCreated(), result.isPurgeOnNoConsumers(), result.getRoutingType(), result.getMaxConsumers(), result.isExclusive(), result.isLastValue(), result.getDefaultConsumerWindowSize());
    --- End diff --
   
    By the way there is no need for a new version with this change if the new param is added at the end and is nullable (aka Integer) old clients simply wont read it and new ones will, for new clients they just need to check if still readable bytes to keep client compatible to older broker. See last change when we added exclusive how it was done.
   
    I would request this change before merge btw.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2191: ARTEMIS-1987 - Add consumer window size to Add...

wy96f
In reply to this post by wy96f
Github user cshannon commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2191
 
    Ok latest changes have been pushed which includes removing the new version and using a nullable integer instead


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2191: ARTEMIS-1987 - Add consumer window size to Add...

wy96f
In reply to this post by wy96f
Github user cshannon commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2191
 
    @clebertsuconic - Is this ok to merge?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2191: ARTEMIS-1987 - Add consumer window size to Add...

wy96f
In reply to this post by wy96f
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2191
 
    Whats the failure


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2191: ARTEMIS-1987 - Add consumer window size to Add...

wy96f
In reply to this post by wy96f
Github user cshannon commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2191
 
    I had to add a setting to one of the tests so hopefully that fixes it, will find out in a few minutes


---
12