[
https://issues.apache.org/jira/browse/ARTEMIS-3850?focusedWorklogId=779428&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-779428
]
ASF GitHub Bot logged work on ARTEMIS-3850:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 08/Jun/22 11:40
Start Date: 08/Jun/22 11:40
Worklog Time Spent: 10m
Work Description: 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.
Issue Time Tracking
-------------------
Worklog Id: (was: 779428)
Time Spent: 3.5h (was: 3h 20m)
> Add Option to read messages into paging based on sizing and eliminate caching
> -----------------------------------------------------------------------------
>
> Key: ARTEMIS-3850
> URL: https://issues.apache.org/jira/browse/ARTEMIS-3850
> Project: ActiveMQ Artemis
> Issue Type: New Feature
> Affects Versions: 2.22.0
> Reporter: Clebert Suconic
> Assignee: Clebert Suconic
> Priority: Major
> Fix For: 2.24.0
>
> Time Spent: 3.5h
> Remaining Estimate: 0h
>
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
