-
-
Notifications
You must be signed in to change notification settings - Fork 757
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
Non fatal bug in removing lock after archive creation #7937
Comments
Isn't the main issue here that your underlying OS answered |
Indeed it looks like this, but actually its not. Also evidence to the open not being the root problem is that locking and unlocking works multiple times during the archive creation, only fails in final steps. |
See also: #7154 (and also quite some other issues, this seems to be a frequent issue at teardown). |
Yeah, guess it is better to use Just out of curiosity: what filesystem does your borg use to access the repository? |
…gbackup#7937 The intention of LockRoster.modify(key, REMOVE) is to remove self.id. Using set.discard will just ignore it if self.id is not present there anymore. Previously, using set.remove triggered a KeyError that has been frequently seen in tracebacks of teardowns involving Repository.__del__ and Repository.__exit__. Thanks to @herrmanntom for the workaround!
…gbackup#7937 The intention of LockRoster.modify(key, REMOVE) is to remove self.id. Using set.discard will just ignore it if self.id is not present there anymore. Previously, using set.remove triggered a KeyError that has been frequently seen in tracebacks of teardowns involving Repository.__del__ and Repository.__exit__. OTOH, at some places, we do not just want to silently ignore, I added REMOVE2 op for these. Thanks to @herrmanntom for the workaround!
…gbackup#7937 The intention of LockRoster.modify(key, REMOVE) is to remove self.id. Using set.discard will just ignore it if self.id is not present there anymore. Previously, using set.remove triggered a KeyError that has been frequently seen in tracebacks of teardowns involving Repository.__del__ and Repository.__exit__. I added a REMOVE2 op to serve one caller that needs to get the KeyError if self.id was not present. Thanks to @herrmanntom for the workaround!
@herrmanntom can you test the code from PR #7939? |
…gbackup#7937 The intention of LockRoster.modify(key, REMOVE) is to remove self.id. Using set.discard will just ignore it if self.id is not present there anymore. Previously, using set.remove triggered a KeyError that has been frequently seen in tracebacks of teardowns involving Repository.__del__ and Repository.__exit__. I added a REMOVE2 op to serve one caller that needs to get the KeyError if self.id was not present. Thanks to @herrmanntom for the workaround!
LockRoster.modify: no KeyError if element was already gone, fixes #7937
…ster LockRoster.modify: no KeyError if element was already gone, fixes #7937
Its gvfs mount of smb share. So indeed its a bit "weird", which is why I investigated the OS Error in the first place - I also thought its some problem of samba or gvfs. |
Sorry, I only had time now. After the proposed PR I still get unlocking issues:
However, I have to say I only applied the changes from PR #7939 to my local copy. I did not really checkout entire 1.2.x for logistical reasons. |
@herrmanntom interesting. guess that confirms my earlier suspicion that So guess that is either a bug in gvfs or smbfs. You don't need to checkout 1.2-maint branch btw, there recently was a 1.2.7 release which also contains the change. |
@ThomasWaldmann: |
Huh, that's strange - I don't think there were other changes related to your issue. Maybe it only happens sometimes or under some other circumstances? |
Hi,
I run borg 1.2.6 like this:
borg create --verbose --filter AME --list --stats --show-rc --compression auto,lzma,6 --noflags --noacls --noxattrs --exclude-caches "::${BORG_BACKUP_NAME_PREFIX}$(date '+%Y-%m-%d_%H-%M-%S')" "${BORG_BACKUP_SOURCE}" --exclude-from "${BORG_BACKUP_EXCLUDE_LIST}"
(with BORG_REPO and BORG_PASSPHRASE set in environment)
and after most backups I get this non fatal exception and rc 2:
after some local messing around with the borg python files I found that the lock here gets "removed" twice:
borg/src/borg/locking.py
Line 316 in 2cefe8f
When looking at the trace back the last superfluous unlock comes from error handler in
borg/src/borg/archiver.py
Line 5361 in 2cefe8f
As workaround in locking.py#L316 just replaced the
elements.remove(self.id)
with
elements.discard(self.id)
I would have expected to now see the issue that triggered the exception handling in archiver.py, but I get no (visible) exceptions anymore.
The text was updated successfully, but these errors were encountered: