[GitHub] [activemq-artemis] asfgit closed pull request #2859: ARTEMIS-2513 Large message's copy may be interfered by other threads
asfgit closed pull request #2859: ARTEMIS-2513 Large message's copy may be interfered by other threads URL: https://github.com/apache/activemq-artemis/pull/2859 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on issue #2859: ARTEMIS-2513 Large message's copy may be interfered by other threads
clebertsuconic commented on issue #2859: ARTEMIS-2513 Large message's copy may be interfered by other threads URL: https://github.com/apache/activemq-artemis/pull/2859#issuecomment-541887630 @gaohoward can I keep this open for 1 or 2 days? I'm doing some work on large messages and I want to make sure why we have that code to close or keep files open. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #2859: ARTEMIS-2513 Large message's copy may be interfered by other threads
clebertsuconic commented on a change in pull request #2859: ARTEMIS-2513 Large
message's copy may be interfered by other threads
URL: https://github.com/apache/activemq-artemis/pull/2859#discussion_r334639212
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/LargeServerMessageImpl.java
##
@@ -368,52 +368,48 @@ public Message copy(final long newID) {
try {
LargeServerMessage newMessage =
storageManager.createLargeMessage(newID, this);
- boolean originallyOpen = file != null && file.isOpen();
+ //clone a SequentialFile to avoid concurrent access
+ ensureFileExists(false);
+ SequentialFile cloneFile = file.cloneFile();
- validateFile();
-
- byte[] bufferBytes = new byte[100 * 1024];
-
- ByteBuffer buffer = ByteBuffer.wrap(bufferBytes);
+ try {
+byte[] bufferBytes = new byte[100 * 1024];
- long oldPosition = file.position();
+ByteBuffer buffer = ByteBuffer.wrap(bufferBytes);
- if (!file.isOpen()) {
-file.open();
- }
- file.position(0);
-
- for (;;) {
-// The buffer is reused...
-// We need to make sure we clear the limits and the buffer before
reusing it
-buffer.clear();
-int bytesRead = file.read(buffer);
-
-byte[] bufferToWrite;
-if (bytesRead <= 0) {
- break;
-} else if (bytesRead == bufferBytes.length &&
!this.storageManager.isReplicated()) {
- // ARTEMIS-1220: We cannot reuse the same buffer if it's
replicated
- // otherwise there could be another thread still using the
buffer on a
- // replication.
- bufferToWrite = bufferBytes;
-} else {
- bufferToWrite = new byte[bytesRead];
- System.arraycopy(bufferBytes, 0, bufferToWrite, 0, bytesRead);
+if (!cloneFile.isOpen()) {
+ cloneFile.open();
}
-newMessage.addBytes(bufferToWrite);
-
-if (bytesRead < bufferBytes.length) {
- break;
+cloneFile.position(0);
+
+for (;;) {
+ // The buffer is reused...
+ // We need to make sure we clear the limits and the buffer
before reusing it
+ buffer.clear();
+ int bytesRead = cloneFile.read(buffer);
+
+ byte[] bufferToWrite;
+ if (bytesRead <= 0) {
+ break;
+ } else if (bytesRead == bufferBytes.length &&
!this.storageManager.isReplicated()) {
+ // ARTEMIS-1220: We cannot reuse the same buffer if it's
replicated
+ // otherwise there could be another thread still using the
buffer on a
+ // replication.
+ bufferToWrite = bufferBytes;
+ } else {
+ bufferToWrite = new byte[bytesRead];
+ System.arraycopy(bufferBytes, 0, bufferToWrite, 0,
bytesRead);
+ }
+
+ newMessage.addBytes(bufferToWrite);
+
+ if (bytesRead < bufferBytes.length) {
+ break;
+ }
}
- }
-
- file.position(oldPosition);
-
- if (!originallyOpen) {
-file.close(false);
-newMessage.getFile().close();
+ } finally {
+cloneFile.close();
}
Review comment:
@wy96f I'm doing some work with large messages, and I want to double check
that as well.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] jbertram commented on issue #2850: ARTEMIS-2504 implement retroactive addresses
jbertram commented on issue #2850: ARTEMIS-2504 implement retroactive addresses URL: https://github.com/apache/activemq-artemis/pull/2850#issuecomment-541866699 I'm not sure what you mean by "destructive testing." There are a number of tests on the PR one of which specifically tests restarting the broker, although there aren't any testing failover. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #2851: ARTEMIS-2503 Improve wildcards for the roles access match
clebertsuconic commented on a change in pull request #2851: ARTEMIS-2503
Improve wildcards for the roles access match
URL: https://github.com/apache/activemq-artemis/pull/2851#discussion_r334625086
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/JMXAccessControlList.java
##
@@ -25,12 +25,22 @@
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
+import org.apache.activemq.artemis.core.config.WildcardConfiguration;
+import org.apache.activemq.artemis.core.settings.HierarchicalRepository;
+import
org.apache.activemq.artemis.core.settings.impl.HierarchicalObjectRepository;
+
public class JMXAccessControlList {
private Access defaultAccess = new Access("*");
- private Map domainAccess = new HashMap<>();
+ private HierarchicalRepository domainAccess;
private ConcurrentHashMap> whitelist = new
ConcurrentHashMap<>();
+ public JMXAccessControlList() {
+ WildcardConfiguration domainAccessWildcardConfiguration = new
WildcardConfiguration();
Review comment:
@michaelandrepearce what do you suggest? merge management and broker.xml?
That's certainly desirable. but I don't think we can do it before a 3.0.
management is on a different domain from queues and addresses. it's about
object names.. not queue names.. hence something simpler would certainly apply
here.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] jbertram commented on a change in pull request #2851: ARTEMIS-2503 Improve wildcards for the roles access match
jbertram commented on a change in pull request #2851: ARTEMIS-2503 Improve
wildcards for the roles access match
URL: https://github.com/apache/activemq-artemis/pull/2851#discussion_r334624011
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/JMXAccessControlList.java
##
@@ -25,12 +25,22 @@
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
+import org.apache.activemq.artemis.core.config.WildcardConfiguration;
+import org.apache.activemq.artemis.core.settings.HierarchicalRepository;
+import
org.apache.activemq.artemis.core.settings.impl.HierarchicalObjectRepository;
+
public class JMXAccessControlList {
private Access defaultAccess = new Access("*");
- private Map domainAccess = new HashMap<>();
+ private HierarchicalRepository domainAccess;
private ConcurrentHashMap> whitelist = new
ConcurrentHashMap<>();
+ public JMXAccessControlList() {
+ WildcardConfiguration domainAccessWildcardConfiguration = new
WildcardConfiguration();
Review comment:
These use-cases seem pretty different to me. The wildcard configuration in
broker.xml is for address matching which is hierarchical in nature and
therefore requires separating "words", matching individual and groups of words,
etc. The "match" performed for keys defined in management.xml is basically just
a **prefix**. It's not hierarchical and doesn't even use regular expressions.
It's *very* simple and IMO it should stay that way.
> If i need to secure a wildcard of address or queues on broker then no
doubt i need to secure the same pattern from admin pov to.
I'm not sure is "no doubt" about that. Use cases for the security of actual
messaging clients and management users can vary quite a bit. In any case, the
same kind of matching isn't possible in management.xml as it is in broker.xml
either before or after this PR. The use-case for this PR is just allowing a
simple prefixing for the match "key" just like is available for the access
"method" field. The syntax is the same. It's just allowing it in a new field. I
don't see any issue with that.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] asfgit closed pull request #2864: ARTEMIS-2519: Use proper enum type inside ActiveMQUnexpectedRoutingTypeForAddress
asfgit closed pull request #2864: ARTEMIS-2519: Use proper enum type inside ActiveMQUnexpectedRoutingTypeForAddress URL: https://github.com/apache/activemq-artemis/pull/2864 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce edited a comment on issue #2850: ARTEMIS-2504 implement retroactive addresses
michaelandrepearce edited a comment on issue #2850: ARTEMIS-2504 implement retroactive addresses URL: https://github.com/apache/activemq-artemis/pull/2850#issuecomment-541822492 So im getting odd behaviour after restarts and failovers. Trying to work out what is going on. Have you done much destructive testing on this? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #2850: ARTEMIS-2504 implement retroactive addresses
michaelandrepearce commented on issue #2850: ARTEMIS-2504 implement retroactive addresses URL: https://github.com/apache/activemq-artemis/pull/2850#issuecomment-541822492 So im getting odd behaviour after restarts. Trying to work out what is going on. Have you done much destructive testing on this? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #2851: ARTEMIS-2503 Improve wildcards for the roles access match
michaelandrepearce commented on a change in pull request #2851: ARTEMIS-2503
Improve wildcards for the roles access match
URL: https://github.com/apache/activemq-artemis/pull/2851#discussion_r334593077
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/JMXAccessControlList.java
##
@@ -25,12 +25,22 @@
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
+import org.apache.activemq.artemis.core.config.WildcardConfiguration;
+import org.apache.activemq.artemis.core.settings.HierarchicalRepository;
+import
org.apache.activemq.artemis.core.settings.impl.HierarchicalObjectRepository;
+
public class JMXAccessControlList {
private Access defaultAccess = new Access("*");
- private Map domainAccess = new HashMap<>();
+ private HierarchicalRepository domainAccess;
private ConcurrentHashMap> whitelist = new
ConcurrentHashMap<>();
+ public JMXAccessControlList() {
+ WildcardConfiguration domainAccessWildcardConfiguration = new
WildcardConfiguration();
Review comment:
So my concern is trying to automate and unify the config the fact already
slightly snowflaked gives me a massive headache as is. Really don't need
further differing config. If we want to enchance the wildcards in management i
feel strongly we need to align it.
E.g. either leave as is, or any new more enhanced stuff needs to be aligned.
I see no reason to have a differing approaches. If i need to secure a wildcard
of address or queues on broker then no doubt i need to secure the same pattern
from admin pov to. Keeping it all aligned just makes it better and simpler to
operate
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #2851: ARTEMIS-2503 Improve wildcards for the roles access match
michaelandrepearce commented on a change in pull request #2851: ARTEMIS-2503
Improve wildcards for the roles access match
URL: https://github.com/apache/activemq-artemis/pull/2851#discussion_r334593077
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/JMXAccessControlList.java
##
@@ -25,12 +25,22 @@
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
+import org.apache.activemq.artemis.core.config.WildcardConfiguration;
+import org.apache.activemq.artemis.core.settings.HierarchicalRepository;
+import
org.apache.activemq.artemis.core.settings.impl.HierarchicalObjectRepository;
+
public class JMXAccessControlList {
private Access defaultAccess = new Access("*");
- private Map domainAccess = new HashMap<>();
+ private HierarchicalRepository domainAccess;
private ConcurrentHashMap> whitelist = new
ConcurrentHashMap<>();
+ public JMXAccessControlList() {
+ WildcardConfiguration domainAccessWildcardConfiguration = new
WildcardConfiguration();
Review comment:
So my concern is trying to automate and unify the config the fact already
slightly snowflaked gives me a massive headache as is. Really don't need
further differing config. If we want to enchance the wildcards in management i
feel strongly we need to align it.
E.g. either leave as is, or any new more enhanced stuff needs to be aligned.
I see no reason to have a differing approaches.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #2851: ARTEMIS-2503 Improve wildcards for the roles access match
michaelandrepearce commented on a change in pull request #2851: ARTEMIS-2503
Improve wildcards for the roles access match
URL: https://github.com/apache/activemq-artemis/pull/2851#discussion_r334593077
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/JMXAccessControlList.java
##
@@ -25,12 +25,22 @@
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
+import org.apache.activemq.artemis.core.config.WildcardConfiguration;
+import org.apache.activemq.artemis.core.settings.HierarchicalRepository;
+import
org.apache.activemq.artemis.core.settings.impl.HierarchicalObjectRepository;
+
public class JMXAccessControlList {
private Access defaultAccess = new Access("*");
- private Map domainAccess = new HashMap<>();
+ private HierarchicalRepository domainAccess;
private ConcurrentHashMap> whitelist = new
ConcurrentHashMap<>();
+ public JMXAccessControlList() {
+ WildcardConfiguration domainAccessWildcardConfiguration = new
WildcardConfiguration();
Review comment:
So my concern is trying to automate and unify the config the fact already
slightly snowflaked gives me a massive headache as is. Really don't need
further differing config. If we want to enchance the wildcards in management i
feel strongly we need to align it
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #2851: ARTEMIS-2503 Improve wildcards for the roles access match
clebertsuconic commented on a change in pull request #2851: ARTEMIS-2503
Improve wildcards for the roles access match
URL: https://github.com/apache/activemq-artemis/pull/2851#discussion_r334548567
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/JMXAccessControlList.java
##
@@ -25,12 +25,22 @@
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
+import org.apache.activemq.artemis.core.config.WildcardConfiguration;
+import org.apache.activemq.artemis.core.settings.HierarchicalRepository;
+import
org.apache.activemq.artemis.core.settings.impl.HierarchicalObjectRepository;
+
public class JMXAccessControlList {
private Access defaultAccess = new Access("*");
- private Map domainAccess = new HashMap<>();
+ private HierarchicalRepository domainAccess;
private ConcurrentHashMap> whitelist = new
ConcurrentHashMap<>();
+ public JMXAccessControlList() {
+ WildcardConfiguration domainAccessWildcardConfiguration = new
WildcardConfiguration();
Review comment:
I'm concerned about users abusing the configuration on management, things
not working later.. and having to answer to user forum's / customer issues...
etc...
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #2851: ARTEMIS-2503 Improve wildcards for the roles access match
clebertsuconic commented on a change in pull request #2851: ARTEMIS-2503
Improve wildcards for the roles access match
URL: https://github.com/apache/activemq-artemis/pull/2851#discussion_r334547881
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/JMXAccessControlList.java
##
@@ -25,12 +25,22 @@
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
+import org.apache.activemq.artemis.core.config.WildcardConfiguration;
+import org.apache.activemq.artemis.core.settings.HierarchicalRepository;
+import
org.apache.activemq.artemis.core.settings.impl.HierarchicalObjectRepository;
+
public class JMXAccessControlList {
private Access defaultAccess = new Access("*");
- private Map domainAccess = new HashMap<>();
+ private HierarchicalRepository domainAccess;
private ConcurrentHashMap> whitelist = new
ConcurrentHashMap<>();
+ public JMXAccessControlList() {
+ WildcardConfiguration domainAccessWildcardConfiguration = new
WildcardConfiguration();
Review comment:
@michaelandrepearce my personal opinion here: can't we keep this simple..
meaning. no configuration of wildcard for management?
I see this can be useful on queues.. etc.. but on management, if we could
keep it simple??
I mean... management and broker should be kept independent.. I wouldn't load
something from broker.xml to change semantics on management...
And if you require the wildcard configuration, then you need testing..and
more moving parts bound to fail and document...
I would prefer to keep it simpler?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #2864: ARTEMIS-2519: Use proper enum type inside ActiveMQUnexpectedRoutingTypeForAddress
clebertsuconic commented on a change in pull request #2864: ARTEMIS-2519: Use
proper enum type inside ActiveMQUnexpectedRoutingTypeForAddress
URL: https://github.com/apache/activemq-artemis/pull/2864#discussion_r334544613
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/ActiveMQUnexpectedRoutingTypeForAddress.java
##
@@ -22,10 +22,10 @@
public final class ActiveMQUnexpectedRoutingTypeForAddress extends
ActiveMQException {
public ActiveMQUnexpectedRoutingTypeForAddress() {
- super(ActiveMQExceptionType.MAX_CONSUMER_LIMIT_EXCEEDED);
Review comment:
oops
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] wy96f commented on issue #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)
wy96f commented on issue #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN) URL: https://github.com/apache/activemq-artemis/pull/2845#issuecomment-541602887 @franz1981 Ok, I see your points of views. Maybe we can try with a custom estimator :) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
