gemmellr commented on code in PR #4101:
URL: https://github.com/apache/activemq-artemis/pull/4101#discussion_r892248712
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/PagedReferenceImpl.java:
##########
@@ -50,14 +47,24 @@ public class PagedReferenceImpl extends
LinkedListImpl.Node<PagedReferenceImpl>
private int messageEstimate = -1;
+ PagePosition position;
+
+ @Override
+ public PagePosition getPosition() {
+ if (position == null) {
+ position = getPagedMessage().newPositionObject();
+ }
+ return position;
+ }
Review Comment:
It may not be necessary for it to be volatile, or may still be insufficient
that it is...depends what the implications are of two threads creating and
using their own independent PagePosition objects for the same message, either
because they either didnt see another threads update of the caching variable,
or because two race to create it and both update the variable, the latter of
which remains possible even if the field is volatile.
I'm not sure how its used overall, but peaking at what newPositionObject()
does, it seems like it just created holding values it gets from the paged
message (for which the getter method this calls to retrieve is synchronized).
The real question would be around whether any of the various values the
position object seems to hold are ever manipulated via the setters, as then
having 'duplicate' objects could be problematic.
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageCursorProviderImpl.java:
##########
@@ -462,7 +285,12 @@ public void cleanup() {
// Then we do some check on eventual pages that can be already
removed but they are away from the streaming
cleanupMiddleStream(depagedPages, depagedPagesSet, cursorList,
minPage, firstPage);
- if (pagingStore.getNumberOfPages() == 0 ||
pagingStore.getNumberOfPages() == 1 &&
pagingStore.getCurrentPage().getNumberOfMessages() == 0) {
+ if (pagingStore.getNumberOfPages() < 0) {
+ new Exception("WHAT???").printStackTrace(System.out);
Review Comment:
I add a "//TODO: delete/update/etc" style comment whenever I add 'hacky
working debug' stuff like this, makes it easy to find later, especially in a
large diff.
--
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]
