gemmellr commented on code in PR #5128:
URL: https://github.com/apache/activemq-artemis/pull/5128#discussion_r1715108779


##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/impl/PagingStoreImpl.java:
##########
@@ -291,6 +291,7 @@ public void applySetting(final AddressSettings 
addressSettings) {
 
       if (pageLimitBytes != null && pageSize > 0) {
          estimatedMaxPages = pageLimitBytes / pageSize;
+         checkNumberOfPages();

Review Comment:
   Calling this here would have it act before start() in the constructor case, 
which seems like it could lead to duplicate logging.
   
   In the non-constructor case it seems like it could cause logging every time 
any [possibly-unrelated] setting changes, which may also not be desirable. 
Maybe it could check if any of those settings changed?



##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/impl/PagingStoreImpl.java:
##########
@@ -343,6 +344,8 @@ private void checkNumberOfPages() {
       if (!isBelowPageLimitBytes()) {
          this.pageFull = true;
          ActiveMQServerLogger.LOGGER.pageFullMaxBytes(storeName, 
numberOfPages, estimatedMaxPages, pageLimitBytes, pageSize);
+      } else {
+         pageLimitReleased();

Review Comment:
   This seems wrong. Just because it isnt over the file-based paging limit, 
doesnt mean it isnt still 'full'; there is a separate message-based limit which 
this would appear to essentially disregard, effectively bypassing it any time 
this method is called...see related use of `checkPageLimit(long 
numberOfMessages)` (next method right below 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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact


Reply via email to