[
https://issues.apache.org/jira/browse/SLING-8854?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16981545#comment-16981545
]
Ashish Chopra commented on SLING-8854:
--------------------------------------
bq. The issue is what you identified in the first place, the retries count is
incremented as a side effect of a getHead operation which does not make sense.
I concur with this understanding.
That's why I wrote that [conflating {{.getHead}} with attempts to process the
queue is
problematic|https://issues.apache.org/jira/browse/SLING-8854?focusedCommentId=16981430&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16981430],
and it needs to be broken down to two separate operations.
# One way to make that happen is what I proposed in the
[patch/PR|https://github.com/apache/sling-org-apache-sling-distribution-core/pull/22]
- adding a new method to the queue so that the processors can indicate there
was a queue processing attempt
# Another would be to keep the behavior of {{.getHead}} as is, and add a
{{.peek}} method to the {{DistributionQueue}} SPI which can be called by
"non-queue-processing" functions in the system.
{quote}
bq. there may be a scenario where updating an item-dequeue-status depends on
queue-implementation (e.g., incrementing a column in the table-entry, if the
queue is implemented as a db-table)
This scenario is hypothetical but even then you could provide your own
implementation.
{quote}
It is not hypothetical - that's pretty much what {{ResourceQueue}} would have
to deal with if it had a processor attached to it (SLING-8853).
To get a consistent queue-entry-process-attempt-count across all instances of
the cluster, one way could be to persist the count alongside the entry. A
{{DistributionQueueProcessor}} should only tell the queue that there was a
processing attempt made. Since persistence to JCR is an implementation detail
of {{ResourceQueue}}, it is {{ResourceQueue}} itself which should be able to
decide how to record the processing-attempts. That's one of the reasons why I'm
not quite on board with making a {{DistributionQueueProcessor}} update a
(transient?) data-structure on the Queue's behalf.
In general, the Queue implementation should be the best judge to decide whether
the Queue is expected to be cluster-aware or not and what do they want to do to
get a cluster-consistent view for processing-attempts. A
queue-processor/queue-provider shouldn't impose that on the queue-impl.
> In-file and In-memory queue-providers for Forward Distribution report
> incorrect value for processing "Attempts"
> ---------------------------------------------------------------------------------------------------------------
>
> Key: SLING-8854
> URL: https://issues.apache.org/jira/browse/SLING-8854
> Project: Sling
> Issue Type: Bug
> Components: Content Distribution
> Affects Versions: Content Distribution Core 0.4.0
> Reporter: Ashish Chopra
> Assignee: Timothee Maret
> Priority: Major
> Attachments:
> 0001-SLING-8854-In-file-and-In-memory-queue-providers-for.patch
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> {{ForwardDistributionAgentFactory}} allows specifying "In-Memory" and
> "In-File" queue-providers [0]. Both these implementations are provided by
> {{SimpleDistributionQueue}} [1].
> While responding to a queue-processing request, the head of the queue is
> fetched to create a {{DistributionQueueEntry}}, which encapsulates a
> {{DistributionQueueItemStatus}}. While retrieving the queue-head-entry, the
> number-of-attempts are also incremented by 1, implying _one more_
> processing-attempt to have happened on a given queue-item. [2]
> The trouble arises from the fact that {{SimpleDistributionQueue#getHead}}
> ends up being called from at least three other places:
> # Queue status Health Check [3], which is a periodic job
> # UI (via {{ExtendedDistributionServiceResourceProvider}}) [4]
> # Distribution Queue status MBean [5]
> Out of these, #1 is most troublesome - because of its periodic nature, the
> number of Attempts for a queue-item that's being processed (or, is blocked)
> will keep on increasing without corresponding dequeue-attempts.
> Due to #2, everytime the queue status is delivered to the UI, the attempt
> would again increase. Similar would be the case of MBean invocations.
> This needs to be addressed. Notably, "Sling Jobs" based queue doesn't suffer
> from this issue because it relies on retry-counts as maintained by Sling Job
> Execution Engine.
> [0]
> https://github.com/apache/sling-org-apache-sling-distribution-core/blob/deb3d2ae33c4f4678c8503091a9fffdbb141e569/src/main/java/org/apache/sling/distribution/agent/impl/ForwardDistributionAgentFactory.java#L258-L262
> [1]
> https://github.com/apache/sling-org-apache-sling-distribution-core/blob/deb3d2ae33c4f4678c8503091a9fffdbb141e569/src/main/java/org/apache/sling/distribution/queue/impl/simple/SimpleDistributionQueue.java
> [2]
> https://github.com/apache/sling-org-apache-sling-distribution-core/blob/deb3d2ae33c4f4678c8503091a9fffdbb141e569/src/main/java/org/apache/sling/distribution/queue/impl/simple/SimpleDistributionQueue.java#L110-L112
> [3]
> https://github.com/apache/sling-org-apache-sling-distribution-core/blob/deb3d2ae33c4f4678c8503091a9fffdbb141e569/src/main/java/org/apache/sling/distribution/monitor/DistributionQueueHealthCheck.java#L113
> [4]
> https://github.com/apache/sling-org-apache-sling-distribution-core/blob/deb3d2ae33c4f4678c8503091a9fffdbb141e569/src/main/java/org/apache/sling/distribution/resources/impl/ExtendedDistributionServiceResourceProvider.java#L158
> [5]
> https://github.com/apache/sling-org-apache-sling-distribution-core/blob/deb3d2ae33c4f4678c8503091a9fffdbb141e569/src/main/java/org/apache/sling/distribution/monitor/impl/DistributionQueueMBeanImpl.java#L62-L91
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
