Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd ref: document releasing of locks in dvc run #860

Closed
efiop opened this issue Dec 13, 2019 · 8 comments · Fixed by #3251
Closed

cmd ref: document releasing of locks in dvc run #860

efiop opened this issue Dec 13, 2019 · 8 comments · Fixed by #3251
Labels
A: docs Area: user documentation (gatsby-theme-iterative) C: guide Content of /doc/user-guide type: discussion Requires active participation to reach a conclusion. type: enhancement Something is not clear, small updates, improvement suggestions

Comments

@efiop
Copy link
Contributor

efiop commented Dec 13, 2019

Context: iterative/dvc#2584

@efiop efiop added the A: docs Area: user documentation (gatsby-theme-iterative) label Dec 13, 2019
@efiop efiop self-assigned this Dec 13, 2019
efiop added a commit to efiop/dvc.org that referenced this issue Dec 16, 2019
jorgeorpinel added a commit to maykulkarni/dvc.org that referenced this issue Dec 18, 2019
…oject (instead of downloading)

per iterative#859 (review),
+ remove "s around 'locked` in some docs (related to iterative#860 and iterative#868).
efiop added a commit to efiop/dvc.org that referenced this issue Dec 22, 2019
efiop added a commit to efiop/dvc.org that referenced this issue Dec 22, 2019
efiop added a commit to efiop/dvc.org that referenced this issue Jan 4, 2020
efiop added a commit to efiop/dvc.org that referenced this issue Jan 4, 2020
efiop added a commit to efiop/dvc.org that referenced this issue Jan 4, 2020
efiop added a commit to efiop/dvc.org that referenced this issue Jan 4, 2020
efiop added a commit to efiop/dvc.org that referenced this issue Jan 4, 2020
efiop added a commit to efiop/dvc.org that referenced this issue Jan 4, 2020
efiop added a commit to efiop/dvc.org that referenced this issue Jan 5, 2020
efiop added a commit to efiop/dvc.org that referenced this issue Jan 6, 2020
efiop added a commit to efiop/dvc.org that referenced this issue Jan 6, 2020
efiop added a commit to efiop/dvc.org that referenced this issue Jan 6, 2020
efiop added a commit to efiop/dvc.org that referenced this issue Jan 6, 2020
@jorgeorpinel jorgeorpinel added p1-important Active priorities to deal within next sprints type: enhancement Something is not clear, small updates, improvement suggestions and removed command-reference labels Jan 18, 2020
@jorgeorpinel jorgeorpinel changed the title document us releasing locks on dvc run cmd ref: document releasing of locks in dvc run Jan 18, 2020
@jorgeorpinel

This comment has been minimized.

@efiop efiop removed their assignment Jan 21, 2020
@jorgeorpinel jorgeorpinel removed the p1-important Active priorities to deal within next sprints label May 6, 2021
@jorgeorpinel
Copy link
Contributor

Hi there! @efiop can you confirm the given context is still up to date and/or copy the parts that now affect repro and exp run? Thanks

@iesahin iesahin added the C: ref Content of /doc/*-reference label Oct 21, 2021
@jorgeorpinel
Copy link
Contributor

Hello again. Is this the what we're talking about? From iterative/dvc#2584 (comment):

Adjusted the schema and made rwlock reentrant, so @rwlocked is now placed on public methods of Stage, which is much nicer than what it was before.

I don't think any user has ever needed to know about this so it sounds like an implementation detail we don't need to document.

Please reopen if I'm incorrect.

@shcheklein
Copy link
Member

I think it's about the user visible behavior - now you could run other DVC commands when dvc run itself is running. Before it was not possible. Since this behavior is more optimistic it is not clear if we need any explicit notes. Only in places (if we have it) where we are clearly stating that no commands could run in parallel, or we discuss locks somewhere.

@jorgeorpinel
Copy link
Contributor

Hmmm well in general the repo is supposed to be locked during any DVC operation or at least that's implied by https://dvc.org/doc/user-guide/troubleshooting#lock-issue . Is that outdated?

We could separately expand on the description of what .dvc/tmp/lock does in https://dvc.org/doc/user-guide/project-structure/internal-files . Other than that I could not find any text mentioning of this limitation so that should be enough to close this.

I started #3251 for this.

@jorgeorpinel jorgeorpinel reopened this Feb 2, 2022
@jorgeorpinel jorgeorpinel added C: guide Content of /doc/user-guide type: discussion Requires active participation to reach a conclusion. and removed C: ref Content of /doc/*-reference labels Mar 10, 2022
@jorgeorpinel
Copy link
Contributor

the repo is supposed to be locked during any DVC operation or at least that's implied by https://dvc.org/doc/user-guide/troubleshooting#lock-issue . Is that outdated?

Hi @dberenbaum (me again 🙂 ) do you know if this info. is outdated? Would be great to get clarification from the core team on what needs to be updated here, if anything (otherwise please close this issue). Thanks

@dberenbaum
Copy link
Contributor

dberenbaum commented Mar 10, 2022

There is a big improvement here for users, because when the user cmd is being executed in a stage, other dvc commands may be run. The repo will still be locked before when dvc is setting up and and after when collecting results of the stage.

However, I don't see any changes needed here because the docs don't address when the repo will or won't be locked, so I think let's close the issue. If we ever put in an in-depth locking guide, we can revisit.

@jorgeorpinel
Copy link
Contributor

Good to know, and sounds good. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: docs Area: user documentation (gatsby-theme-iterative) C: guide Content of /doc/user-guide type: discussion Requires active participation to reach a conclusion. type: enhancement Something is not clear, small updates, improvement suggestions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants