mattrpav commented on code in PR #2233:
URL: https://github.com/apache/activemq/pull/2233#discussion_r3962529646
##########
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) {
+ var formatted = Math.round((double)freePageCount / totalPageCount
* 100d) / 100d;
+ LOG.debug("Skipping compaction, page file freePageRatio {} is less
than "
+ + "configured maxFreePageCompactionRatio {}",
String.format("%,.2f", formatted), maxFreePageCompactionRatio);
+ return;
+ }
+
+ // Get the last sequence of contiguous set of free pages in the file
+ // This block could be either in the middle of the file somewhere or
+ // at the end of the file
+ final Sequence lastFreeSeq = freeList.getTail();
+
+ // Sanity check, should not happen if we have a free page ratio
+ if (lastFreeSeq == null) {
+ LOG.warn("Skipping compaction, no free pages");
+ return;
+ }
+
+ // If we have a block of free pages then check if it is
+ // at the end of the file.
+ //
+ // If the offset of the last free page + the size of a page
+ // (to account for the nextFreePage that was allocated already)
+ // equals the disk size then there are no in use pages after this
block.
+ if (toOffset(lastFreeSeq.getLast()) + pageSize != diskSize) {
+ LOG.debug("Unable to compact, last free page block is not at the
end of the file");
+ return;
+ }
+
+ long minFreePages = Math.round(totalPageCount *
minFreePageCompactionRatio);
+
+ // we must keep a minimum number of free pages so find the point
+ // where we can truncate without removing too many pages
+ long maxPagesToTruncate = freePageCount - minFreePages;
+
+ final Sequence freePagesToDelete;
+ // If the block of free pages is larger than the max we need to
truncate
+ // then we can split the sequence to only delete the max
+ if (lastFreeSeq.range() > maxPagesToTruncate) {
+ var last = lastFreeSeq.getLast();
+ var updatedFirst = (last - maxPagesToTruncate) + 1;
+ freePagesToDelete = new Sequence(updatedFirst, last);
+ } else {
+ freePagesToDelete = lastFreeSeq;
+ }
+
+ // sync on writes to block the async thread from trying to write while
+ // we are updating the file and truncating
+ synchronized (writes) {
Review Comment:
What is the interplay here b/w locking on the writes and the compact()
already running within the index lock?
If it is in the indexLock, then do the writes need to be synchronized here?
If so, thoughts on it moving up sooner since the threshold checks could rapidly
change (ie async producer)?
--
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
