Ensure that the reconcile queue tracks processing items

[ghost]

Description

This commit adds some new state and functions to the orchestration queue so that it keeps track of Dequeued items until that item is marked as Done by the person who Dequeued it.

This information is used to prevent items from being re-queued while the system is processing an item because the poller feeding the queue may pick up an item which is in the process of being updated.

This also makes it safe to have multiple reconciler threads being fed from a single queue because we have a guarentee that the same item isn't in the queue twice and therefore we can't end up with mutiple reconcilers trying to process the same item.

Type of Change

[ ] Bug Fix
[ ] New Feature
[ ] Breaking Change
[X] Refactor
[ ] Documentation
[ ] Other (please describe)

Checklist

• I have read the contributing guidelines
• Existing issues have been referenced (where applicable)
• I have verified this change is not present in other open pull requests
• Functionality is documented
• All code style checks pass
• New code contribution is covered by automated tests
• All new and existing tests pass

[FrimIdan]

This information is used to prevent items from being re-queued while the system is processing an item because the poller feeding the queue may pick up an item which is in the process of being updated.

@sambetts-cisco Not sure I understand, why that an item that was de-queued will be re-queued?
Also, why we are not removing it from the inqueue once done is called? why we need both inqueue and processing?

[ghost]

@sambetts-cisco Not sure I understand, why that an item that was de-queued will be re-queued? Also, why we are not removing it from the inqueue once done is called? why we need both inqueue and processing?

The poller searching for items in specific states continues to run while the reconciler processes an item, which can result in the poller re-adding the item which is being processed to the end of the queue, because the queue doesn't know about this item any longer.

By keeping track of the items which are being processed we can avoid readding these items and avoid an extra reconcile when we have a single reconciler, if we enable parallel reconcilers for some objects then its required to avoid two reconcilers trying to reconcile the same item.

I original implemented this re-using the inqueue map, but that broke the behaviour I wanted for the "Has" function which is meant to check if something is in the queue, so I needed to maintain the list of processing items separately which is why I added the processing map.

[FrimIdan]

I original implemented this re-using the inqueue map, but that broke the behaviour I wanted for the "Has" function which is meant to check if something is in the queue, so I needed to maintain the list of processing items separately which is why I added the processing map.

Has returns true if items is currently in the queue

So the Has is for checking if the item is in the queue or in processing or both? From the code I see that it both in the queue and in processing so not sure, again, why we can't keep it in the queue until it was done processing.

[ghost]

So the Has is for checking if the item is in the queue or in processing or both? From the code I see that it both in the queue and in processing

Has is for checking if an item is in the queue itself, dequeued items would return false even though they continue to be processed.

Dequeue removes the item from "inqueue" https://github.com/openclarity/vmclarity/pull/314/files#diff-c8aea9db7e7fbce1cfcb5e8ca00dabd75079958e9fa3409f2bbc9e5093f70e1cL86 so an item is only ever in "inqueue" if its in the queue slice.

[FrimIdan]

Has is for checking if an item is in the queue itself, dequeued items would return false even though they continue to be processed.

	_, inQueue := q.inqueue[item]
	_, isProcessing := q.processing[item]
	return inQueue || isProcessing

Maybe i'm missing something but if the item continue to be processed the Has will return true.

[ghost]

Maybe i'm missing something but if the item continue to be processed the Has will return true.

face plam I must have changed that at some point during development and now I don't remember why...

[ghost]

face plam I must have changed that at some point during development and now I don't remember why...

@FrimIdan I remembered why I track processing items separately! Its to prevent the inqueue map and q.queue's state getting out of sync.
If you use the same map to track the queue and the processing items, then its possible to call Done(item) removing it from inqueue even though its not been dequeued yet. So then Has(..) will return false even though the item is still in the queue slice. If Done operates only on a separate slice then it can't affect the Dequeue/Enqueue behaviour.

I'll add a comment.