[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #2775: ARTEMIS-2435 Configuration on device-block-size through CLI / broker.xml
michaelpearce-gain commented on a change in pull request #2775: ARTEMIS-2435
Configuration on device-block-size through CLI / broker.xml
URL: https://github.com/apache/activemq-artemis/pull/2775#discussion_r309088480
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/utils/XMLConfigurationUtil.java
##
@@ -44,7 +44,6 @@ public static final Double getDouble(final Element e,
validator.validate(name, val);
return val;
} else {
- validator.validate(name, def);
Review comment:
Surely though it would be valid for someone to set a default value, as such
whilst a bit backwards, this would actually be (from unit testing) validate
that the validators set allow for the default value. If they dont and they
fail, then the validator isnt correct.
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] michaelpearce-gain commented on a change in pull request #2775: ARTEMIS-2435 Configuration on device-block-size through CLI / broker.xml
michaelpearce-gain commented on a change in pull request #2775: ARTEMIS-2435 Configuration on device-block-size through CLI / broker.xml URL: https://github.com/apache/activemq-artemis/pull/2775#discussion_r309092043 ## File path: artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java ## @@ -266,10 +266,12 @@ @Option(name = "--no-fsync", description = "Disable usage of fdatasync (channel.force(false) from java nio) on the journal") private boolean noJournalSync; + @Option(name = "--device-block-size", description = "The block size by the device, default at 4096.") Review comment: should this be journal-device-block-size. e.g. what occurs in future where want to tune the others, and they maybe using other disks with other block sizes, might be best to namespace this now, so in future doesnt cause any issue 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] michaelpearce-gain commented on a change in pull request #2775: ARTEMIS-2435 Configuration on device-block-size through CLI / broker.xml
michaelpearce-gain commented on a change in pull request #2775: ARTEMIS-2435 Configuration on device-block-size through CLI / broker.xml URL: https://github.com/apache/activemq-artemis/pull/2775#discussion_r309092043 ## File path: artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java ## @@ -266,10 +266,12 @@ @Option(name = "--no-fsync", description = "Disable usage of fdatasync (channel.force(false) from java nio) on the journal") private boolean noJournalSync; + @Option(name = "--device-block-size", description = "The block size by the device, default at 4096.") Review comment: should this be journal-device-block-size, as like the actual field name. e.g. what occurs in future where want to tune the others, and they maybe using other disks with other block sizes, might be best to namespace this now, so in future doesnt cause any issue 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 #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
wy96f commented on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2750#issuecomment-516807021 Pushed to change PageReader per queue. The perf result is below: 1. Running in 51MB size page and 1 page cache in the case of 100 multicast queues. 19000 msg/s received and 9500 msg/s sent vs. 16000 msg/s received and 11000 msg/s sent with previous PageReader cache. consumer tps:  producer tps:  2. Running in 5MB size page and 100 page cache in the case of 100 multicast queues. 16000msg/s received and 8200 msg/s sent vs. 15000 msg/s received and 8500 msg/s sent with previous PageReader cache. consumer tps:  producer tps:  3. Running in 51MB size page and 1 page cache in the case of 1 queue. 16000 msg/s received and 29000 msg/s sent vs. 13500 msg/s received and 3 msg/s sent with previous PageReader cache. consumer tps:  producer tps:  Generally speaking, the consumer tps increase a little and producer tps reduce less, with total tps increasing by about 1000. It's to be noted that there was frequent file opening/closing in second test scenario(5MB size page and 100 page cache). I added some logging and saw two subsequent deliver was scheduled a few seconds after depage. In the case that message of PagedReference was cleared, calling getPriority/checkExpired in deliver() would trigger reading message. Due to the small page file, reading messages were more likely to hit page before the current cursor. Given PageReader before current cursor was closed, this would cause constantly page file open/close(log showed approximately 6000 times per second in average). In first test scenario(51MB size page and 1 page cache), approximately 700 times per second in average of file open/closing occurred. 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 #2775: ARTEMIS-2435 Configuration on device-block-size through CLI / broker.xml
clebertsuconic commented on a change in pull request #2775: ARTEMIS-2435
Configuration on device-block-size through CLI / broker.xml
URL: https://github.com/apache/activemq-artemis/pull/2775#discussion_r309277549
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/utils/XMLConfigurationUtil.java
##
@@ -44,7 +44,6 @@ public static final Double getDouble(final Element e,
validator.validate(name, val);
return val;
} else {
- validator.validate(name, def);
Review comment:
@michaelandrepearce I'm confused if you want me to just add a test or add
the validator back?
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 #2775: ARTEMIS-2435 Configuration on device-block-size through CLI / broker.xml
clebertsuconic commented on a change in pull request #2775: ARTEMIS-2435 Configuration on device-block-size through CLI / broker.xml URL: https://github.com/apache/activemq-artemis/pull/2775#discussion_r309277809 ## File path: artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java ## @@ -266,10 +266,12 @@ @Option(name = "--no-fsync", description = "Disable usage of fdatasync (channel.force(false) from java nio) on the journal") private boolean noJournalSync; + @Option(name = "--device-block-size", description = "The block size by the device, default at 4096.") Review comment: ok.. I will send another PR with journal-* 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] michaelpearce-gain commented on a change in pull request #2775: ARTEMIS-2435 Configuration on device-block-size through CLI / broker.xml
michaelpearce-gain commented on a change in pull request #2775: ARTEMIS-2435
Configuration on device-block-size through CLI / broker.xml
URL: https://github.com/apache/activemq-artemis/pull/2775#discussion_r309281215
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/utils/XMLConfigurationUtil.java
##
@@ -44,7 +44,6 @@ public static final Double getDouble(final Element e,
validator.validate(name, val);
return val;
} else {
- validator.validate(name, def);
Review comment:
Validator back
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 opened a new pull request #2776: NO-JIRA Speeding up tests
clebertsuconic opened a new pull request #2776: NO-JIRA Speeding up tests URL: https://github.com/apache/activemq-artemis/pull/2776 When it comes to the testsuite, we don't need timed buffers kicking too much. This should bring some speed for our testsuite 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 #2775: ARTEMIS-2435 Configuration on device-block-size through CLI / broker.xml
clebertsuconic commented on a change in pull request #2775: ARTEMIS-2435
Configuration on device-block-size through CLI / broker.xml
URL: https://github.com/apache/activemq-artemis/pull/2775#discussion_r309519331
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/utils/XMLConfigurationUtil.java
##
@@ -44,7 +44,6 @@ public static final Double getDouble(final Element e,
validator.validate(name, val);
return val;
} else {
- validator.validate(name, def);
Review comment:
@michaelandrepearce well.. I don't see a point on that.. really.
It's moot, it's dead code.
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 #2775: ARTEMIS-2435 Configuration on device-block-size through CLI / broker.xml
clebertsuconic commented on a change in pull request #2775: ARTEMIS-2435
Configuration on device-block-size through CLI / broker.xml
URL: https://github.com/apache/activemq-artemis/pull/2775#discussion_r309519331
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/utils/XMLConfigurationUtil.java
##
@@ -44,7 +44,6 @@ public static final Double getDouble(final Element e,
validator.validate(name, val);
return val;
} else {
- validator.validate(name, def);
Review comment:
@michaelandrepearce well.. I don't see a point on that.. really.
It's moot, it's dead code.
I know that piece of code from the day it was written. it was just an
utility class, and the default value was not meant to be evaluated.
If you select your default value, it sure is intended to be returned as a
value, no need for validations.
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
