[jira] [Commented] (SLING-8854) In-file and In-memory queue-providers for Forward Distribution report incorrect value for processing "Attempts"

["Ashish Chopra (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SLING-8854?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16987745#comment-16987745 ] 

Ashish Chopra commented on SLING-8854:
--------------------------------------

[~marett], comments on SLING-8743 indicate that separating the implementation of {{DistributionQueueProvider}} and {{DistributionQueue}} with that of a {{DistributionQueueProcessor}} is the correct semantic.
(the implementation of the queue is expected to reside in one bundle - such as {{sling.distribution.core}}, whereas its processing is expected to exist in another bundle).

Given above, we shouldn't think of the queue-processor to be coupled with queue and queue-providers and thus can't expect the queue-processors to work in cohorts with queue and queue-providers to maintain the processing attempts.

IMHO the resolution of this issue would be to:
* either change the behaviour of {{DistributionQueue.getHead}} such that it doesn't increment the processing count automatically and add a new method to the SPI which {{DistributionQueueProcessor}} can invoke to let the queue capture a processing attempt ([approach #1 here|https://issues.apache.org/jira/browse/SLING-8854?focusedCommentId=16982396&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16982396])
** fwiw, this also aligns with the opinion you expressed that [{{DistributionQueue.getHead}} has no business incrementing processing count and should be side-effect free|https://issues.apache.org/jira/browse/SLING-8854?focusedCommentId=16982155&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16982155].
* or add a new {{DistributionQueue.peek}} for introspection, expected to be used by components interested in inspecting and reporting queue-statuses ([approach #2 here|https://issues.apache.org/jira/browse/SLING-8854?focusedCommentId=16982396&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16982396])
** this keeps the behaviour of {{DistributionQueue.getHead}} intact - if required for backward-compatibility reasons - but IMHO is slightly more confusing than the first option.

IMVHO approach which expects queue-provider, queue and queue-processors is too much coupling and severely reduces the fluency of the API.

Looking forward to your feedback on the alternative PRs [I shared here|https://issues.apache.org/jira/browse/SLING-8854?focusedCommentId=16982396&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16982396]. Thanks!

> 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: 20m
>  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)