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

LFS lock client has race conditions #81

Open
ConorVernon opened this issue Feb 7, 2023 · 11 comments
Open

LFS lock client has race conditions #81

ConorVernon opened this issue Feb 7, 2023 · 11 comments
Labels
bug Something isn't working

Comments

@ConorVernon
Copy link

ConorVernon commented Feb 7, 2023

Content that resides in the __ExternalActors__ directory used by the One File Per Actor system appears to occasionally desync between their edited and locked status. Seems particularly prone to error with file deletions.

When this happens, git will report no files to check in, yet when running git lfs locks several locks can be seen on External Actors, requiring manual unlocking.

This leads to some interesting desync issues for our team, where one person submits a series of changes with no apparrent problems, their git commit includes the changed files. Yet only the unlocked changes show up on the remote and other machines.

This is something we've noticed now because they changed a lot of files at once, but it would explain several other instances where we've seen small changes not properly show up.

Are there any workarounds for these issues besides catching them when they happen and manually unlocking? As a side note, are there any plans to add file-locking feedback into the World Outliner?

@mastercoms
Copy link
Member

There might be some errors in the locking/unlocking process that are causing a fatal premature exit. Could you replace the git-lfs binaries in the plugin's root folder with the official ones and see if the problem still remains? Locking will be slower but it may show if the problem is in the source control plugin or the LFS fork we have.

There's no plans to add additional source control feedback besides what is in the editor provided by Epic. Hopefully Epic would be able to assist you in enhancing OFPA's source control display.

@ConorVernon
Copy link
Author

ConorVernon commented Feb 8, 2023

Thanks for responding, I will replace the binaries and attempt to reproduce.

@mastercoms
Copy link
Member

I just remembered, you can't do that directly, I'll make a branch or something with the proper modifications

@ConorVernon
Copy link
Author

ConorVernon commented Feb 9, 2023

Thanks for that! Once that's up I'll work on reproducing it.

It might take me some time though since I'm otherwise fairly busy at the moment. I'm also undecided as to whether to rope the rest of my team into this test (most of them don't normally use source control so they have enough stress trying to work with it as is!). I'll keep this thread updated.

Let me know if there are any specific steps you'd like me to try. I'm currently thinking I'll make a fresh project, create two branches with and without the modified binaries, and attempt to force the issue on both. Does that sound good to you?

@mastercoms
Copy link
Member

mastercoms commented Feb 11, 2023

I've now implemented the change to selectively use your system's version of Git LFS. No need to replace binaries anymore. As seen in this commit, you can define GIT_USE_CUSTOM_LFS 0 to use the system installed version.

I think this bug can manifest the more files you lock/unlock at the same time, if it happens at all. So being able to test it with many OFPA actors may help to discover it.

@ConorVernon
Copy link
Author

Thank you, I'll get to testing that as soon as I can.

@mastercoms
Copy link
Member

mastercoms commented Apr 26, 2023

As a note (to self), it seems like SSH authentication has additional race conditions that should be solved, see #90.

@mastercoms mastercoms changed the title With One File Per Actor, files can get stuck in a locked state LFS lock client has race conditions leading Apr 26, 2023
@mastercoms mastercoms changed the title LFS lock client has race conditions leading LFS lock client has race condition Apr 26, 2023
@mastercoms mastercoms changed the title LFS lock client has race condition LFS lock client has race conditions Apr 26, 2023
@Schroedingers-Cat
Copy link

I've experienced exactly what @ConorVernon has been describing. In my case, the issue was that we were adding the usernames of our GitLab accounts (the ones we use to login) to the Source Control Login window in the text field next to "Use Git LFS". It turned out that we had to use our profile's "Full Name" instead. It's the same as reported by git lfs locks.

The weird thing is that with the wrong name provided, you can submit commits as well as successfully lock files and release file locks for a while. It'll take some time until issues pop up like changed files from the __ExternalActors__ folder not being picked up by the plugin.

Maybe it's possible to detect the desync and show a warning to the user that this is likely due to a wrong LFS lock account name in the settings. Or even better, detect the wrong user name right away.

It could also help to add a label next to the text field because "Use Git LFS" isn't a good description of what should go into the text field. Additionally, since it's auto-populated with the git username from you local config, the field description won't be seen by the users unless they clear it on their own. Currently, it looks like this and I first didn't even note that the text field is actually about the LFS file lock account name:
grafik

@mastercoms
Copy link
Member

Yeah, this is a separate issue that I intend to solve with auto-detection.

@mastercoms mastercoms added the bug Something isn't working label May 5, 2023
@ConorVernon
Copy link
Author

I'm sorry I never got around to properly testing those fixes. Is there any testing that can be done now to help you resolve this?

@Louspirit
Copy link

Louspirit commented Apr 4, 2024

Do you know where to be informed about OFPA's source control display supporting GIT (changelist window)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants