mattrpav commented on code in PR #2233:
URL: https://github.com/apache/activemq/pull/2233#discussion_r4066864369
##########
activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/page/PageFile.java:
##########
@@ -613,6 +645,150 @@ public void flush() throws IOException {
}
}
+ /**
+ * Compact the PageFile if needed. The only supported strategy is
truncation,
+ * but future strategies may be added.
+ * <p>
+ * PageFile is a contiguous block of pages and new pages get allocated
+ * as space is needed. If pages are no longer needed anymore, they are
+ * marked as free so they can be re-used. This normally works well
+ * for workflows that are consistent.
+ * <p>
+ * However, sometimes it's possible to end up with a large number of free
+ * pages and wasted space. An example is an unusual backlog of data on
+ * a destination. This can cause a huge increase in the PageFile size to
+ * track the messages. Once the backlog is gone, the PageFile has a lot
+ * of free pages and is huge and will never shrink.
+ * <p>
+ * Truncation helps in this scenario by simply truncating the file and
+ * removing the excess free pages. This works well because all we need
+ * to do is remove the free pages from the tracking and then simply
+ * set the length of the file to the new size which is nearly instant time,
+ * so there is no noticeable performance impact.
+ * <p>
+ * One major limitation to note is that this strategy only works if the
free pages are
+ * at the end of the file. If the free pages are in the middle of the file
+ * his won't work so we can't shrink the file. This could happen, for
example, if a new
+ * destination was created while thre was a big backlog and it was written
+ * to the index. This situation will hopefully be addressed in a future
+ * compaction update as it is a more complex to handle and will likely
require
+ * a different compaction strategy such as defragging first or rewriting
the file.
+ *
+ * @throws IOException
+ */
+ public void compact() throws IOException {
+ if (compactionStrategy.isNever()) {
+ LOG.debug("Skipping PageFile compaction check, compaction is
disabled.");
+ return;
+ }
+
+ LOG.debug("Beginning PageFile compaction check.");
+
+ // Don't compact if we have not finished free page recovery
+ if (trackingFreeDuringRecovery.get() != null) {
+ LOG.debug("Skipping compaction, async recovery not finished");
+ return;
+ }
+
+ // diskSize computed by checking nextFreePageId
+ long diskSize = getDiskSize();
+
+ // Disk size of the free pages in the page file
+ long freePageCount = getFreePageCount();
+ long totalPageCount = getPageCount();
+
+ // Percentage of pages that are free vs in use
+ double freePageRatio = Math.round((double)freePageCount /
totalPageCount * 100d) / 100d;
+
+ // Only attempt to compact if we have reached the maximum ratio of
free pages
+ // that is configured.
+ var tolerance = .001;
+ if (freePageRatio < maxFreePageCompactionRatio - tolerance) {
Review Comment:
Feels like the rounding could result in fuzzy numbers. Thoughts on using
something simplified:
```
if (freePageCount < totalPageCount * maxFreePageCompactionRatio)
```
--
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
