Skip to content

Conversation

@btovar
Copy link
Member

@btovar btovar commented Jan 20, 2026

If the replica count of a temp file is 0 because of workers disconnecting, we now remove the file from the "needs to replicate" table. This removes a warning when we try to reinstert the file into the table when the cache-update of the recovery task is received, and should make iterating the table a little more efficient.

Proposed Changes

Give an overall description of the changes, along with the context and motivation.
Mention relevant issues and pull requests as needed.

Merge Checklist

The following items must be completed before PRs can be merged.
Check these off to verify you have completed all steps.

  • make test Run local tests prior to pushing.
  • make format Format source code to comply with lint policies. Note that some lint errors can only be resolved manually (e.g., Python)
  • make lint Run lint on source code prior to pushing.
  • Manual Update: Update the manual to reflect user-visible changes.
  • Type Labels: Select a github label for the type: bugfix, enhancement, etc.
  • Product Labels: Select a github label for the product: TaskVine, Makeflow, etc.
  • PR RTM: Mark your PR as ready to merge.

If the replica count of a temp file is 0 because of workers
disconnecting, we now remove the file from the "needs to replicate"
table. This removes a warning when we try to reinstert the file
into the table when the cache-update of the recovery task is received,
and should make iterating the table a little more efficient.
@btovar btovar requested a review from JinZhou5042 January 20, 2026 18:58
@btovar
Copy link
Member Author

btovar commented Jan 20, 2026

The 'table' is a priority queue. Closing and looking for a more efficient solution.

@btovar btovar closed this Jan 20, 2026
@JinZhou5042
Copy link
Member

I guess this isn't necessary? Because if the replica count of a file is 0, then the replication demand will be skipped when it is visited.

@btovar
Copy link
Member Author

btovar commented Jan 20, 2026

@JinZhou5042 It seems that this is fixed in master, but not the current release?

@btovar btovar deleted the no_ht_notice branch January 20, 2026 19:07
@JinZhou5042
Copy link
Member

Yeah, I don't see this as a problem right now.

@btovar
Copy link
Member Author

btovar commented Jan 20, 2026

It is needed when it is a table and not a queue. Otherwise we get a debug message on reinsertion, like:
2026/01/20 14:07:06.48 vine_manager[1698412]error: Failed to insert key temp-rnd-jfglxpqaspvxfbq into hash table.

@JinZhou5042
Copy link
Member

Ah got it!

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