[ https://issues.apache.org/jira/browse/SLING-8854?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16982396#comment-16982396 ]
Ashish Chopra edited comment on SLING-8854 at 11/26/19 11:41 AM: ----------------------------------------------------------------- _(in continuation with discussion summarized [here|https://issues.apache.org/jira/browse/SLING-8854?focusedCommentId=16982155&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16982155])_: hi [~marett], as you had asked for off-JIRA, here are the PRs for all three approaches possible here: * @ashishc's approach #1 (described [here|https://issues.apache.org/jira/browse/SLING-8854?focusedCommentId=16981545&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16981545]): add a {{.recordProcessingAttempt}} method to {{DistributionQueue}} SPI that {{DistributionQueueProcessor}} impls are expected to invoke once they dequeue the queue-head with the intent of processing it ** [https://github.com/apache/sling-org-apache-sling-distribution-core/pull/22] * @ashishc's approach #2 (described [here|https://issues.apache.org/jira/browse/SLING-8854?focusedCommentId=16981545&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16981545]): add a {{.peek}} method to {{DistributionQueue}} SPI. {{DistributionQueue}} impls are not expected to consider this method's invocation as a processing attempt of the queue-head - this method is expected to be invoked by functions which need to introspect the Queue (Health Checks, MBeans, UI, etc.) ** [https://github.com/apache/sling-org-apache-sling-distribution-core/pull/25] * @marett's approach (described [here|https://issues.apache.org/jira/browse/SLING-8854?focusedCommentId=16980314&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16980314]): impls of {{DistributionQueueProvider}}, {{DistributionQueue}} and {{DistributionQueueProcessors}} are expected to manage the state by sharing implementation details amongst them. ** [https://github.com/apache/sling-org-apache-sling-distribution-core/pull/26] was (Author: ashishc): _(in continuation with discussion summarized here)_: hi [~marett], as you had asked for off-JIRA, here are the PRs for all three approaches possible here: * @ashishc's approach #1 (described here): add a {{.recordProcessingAttempt}} method to {{DistributionQueue}} SPI that {{DistributionQueueProcessor}} impls are expected to invoke once they dequeue the queue-head with the intent of processing it ** [https://github.com/apache/sling-org-apache-sling-distribution-core/pull/22] * @ashishc's approach #2 (described here): add a {{.peek}} method to {{DistributionQueue}} SPI. {{DistributionQueue}} impls are not expected to consider this method's invocation as a processing attempt of the queue-head - this method is expected to be invoked by functions which need to introspect the Queue (Health Checks, MBeans, UI, etc.) ** [https://github.com/apache/sling-org-apache-sling-distribution-core/pull/25] * @marett's approach (described here): impls of {{DistributionQueueProvider}}, {{DistributionQueue}} and {{DistributionQueueProcessors}} are expected to manage the state by sharing implementation details amongst them. ** [https://github.com/apache/sling-org-apache-sling-distribution-core/pull/26] > 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)