Skip to content
This repository has been archived by the owner on Mar 9, 2019. It is now read-only.

Added support for no timeout locks on db files #494

Closed
wants to merge 19 commits into from
Closed

Added support for no timeout locks on db files #494

wants to merge 19 commits into from

Conversation

dmoklaf
Copy link

@dmoklaf dmoklaf commented Jan 23, 2016

Currently the bolt library supports timeouts for db file locking, as well as infinite timeouts. This patch adds support for no timeouts at all.

@dmoklaf
Copy link
Author

dmoklaf commented Dec 24, 2016

Hello is there any problem with this patch, or required updates?
Thanks

@benbjohnson
Copy link
Member

@rgeronimi Sorry, I'm just terrible at managing PRs. Can you add a test for this change? Otherwise it lgtm.

@dmoklaf
Copy link
Author

dmoklaf commented Dec 24, 2016

Is ok, I thought there was a code reason :)
I will take some time to add a few tests and I come back to you.
Have a nice Xmas

@dmoklaf
Copy link
Author

dmoklaf commented Aug 29, 2017

Hello, I freed up the time to add the missing test. Unfortunately I was about to add a variant of the exact piece of code removed through
commit b9eb643
(@benbjohnson committed on 27 Dec 2016 )
"The new FCTNL locking does not support multiple locks from the
same process which makes those tests fail. The lock tests have
been removed."

I see 2 alternatives:

  • In line with this commit we drop entirely the objective to test lock-related functionality
  • We use fork a side process to hold the commit

Unfortunately the second alternative requires an external executable (Fork and not ForkExec is not safe to call from within a Go process, as it forgets about Go runtime-related threads in the forked process) which is, depending on how and when tests are run, not trivial.

Should I continue down that second alternative? If yes, I need a separate Go executable which will need to be go-built before the go-test stage begins. Do you see any simpler approach than that?

Thanks

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants