Skip to content

Conversation

@fd0
Copy link
Member

@fd0 fd0 commented Sep 27, 2015

This breaks out some commits from #252 (which was clearly too large to be done in one PR). The code in this PR adds resuming of interrupted backups by regularly writing an index (utilizing the new supersedes field) and reading these when a backup should be resumed.

Closes #59

@fd0 fd0 mentioned this pull request Sep 27, 2015
@fd0
Copy link
Member Author

fd0 commented Oct 14, 2015

@episource I've handled the bug you discovered, thanks again.

@episource
Copy link
Contributor

Is a RWMutex reantrant? The documentation does not say anything about this...

~~Context: ~~
AddInFlight acquires a RW-Lock (mi.inFlight.Lock()) and then calls Has, which tries to acquire a Read-Lock (mi.idxMutex.RLock()). Without RWMutex being reantrant this should fail/dead-lock.

Assuming go locks to be reantrant I tried the same while implementing my draft of StoreInProgress. Due to that I learned the hard way, that my assumption was wrong and Mutex (not RWMutex) is not reantrant.

Edit: Reading my comment again, I noticed that two different lock objects are involved.

Edit: Nothing is locked twice. Everything is alright.

@fd0
Copy link
Member Author

fd0 commented Oct 14, 2015

Uhoh. I'll have a look, thanks again!

@episource
Copy link
Contributor

No, just forget my comment. It's plain wrong.

@fd0
Copy link
Member Author

fd0 commented Oct 14, 2015

Hm, I'd expect that acquiring the r lock while an r/w lock is already held leads to a deadlock...

@fd0
Copy link
Member Author

fd0 commented Oct 14, 2015

Ah, indeed, now I see it.

@fd0
Copy link
Member Author

fd0 commented Oct 14, 2015

Any other comments? Otherwise I'll merge this branch now :)

@episource
Copy link
Contributor

Not from my side...

@fd0
Copy link
Member Author

fd0 commented Oct 14, 2015

cool

@fd0 fd0 merged commit 1dd731f into master Oct 14, 2015
fd0 added a commit that referenced this pull request Oct 14, 2015
@fd0 fd0 deleted the resume-backups branch October 14, 2015 19:53
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.

3 participants