Skip to content

Add reconciliation of sandbox kv store key counts.#6

Open
hodgestar wants to merge 2 commits into
developfrom
feature/issue-6-add-kv-key-count-reconciliation
Open

Add reconciliation of sandbox kv store key counts.#6
hodgestar wants to merge 2 commits into
developfrom
feature/issue-6-add-kv-key-count-reconciliation

Conversation

@hodgestar

Copy link
Copy Markdown
Contributor

No description provided.

@hodgestar

Copy link
Copy Markdown
Contributor Author

Not at all ready for review, although the plan is fairly well documented and a skeleton of solution is in place.

Comment thread vxsandbox/resources/kv.py

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this isn't yet ready for review, but its a bit unclear to me what part the locks play in these steps. Could you explain?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lock is to prevent multiple workers from doing reconciliation work at the same time.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. Missed this bit, sorry:

Only one worker performs reconciliation work at a time. This reduces load on Redis and simplifies the implementation.

This step confused me:

If there is a reconciliation in progress, continue scanning keys from where the work stopped, updating counts as needed.

I thought that meant that another recon can start while another is in progress, and it'd continue work from where the other recon is going to be ending.

Looking at the implementation, it seems like each looping call does a bit of work at a time, and then leaves the rest for the next call. Is that what happens?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants