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.

Sign up for GitHub

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

Jump to bottom

Add advisory locking for POSIX based systems #123

Closed
3 tasks done
ohsayan opened this issue Apr 16, 2021 · 2 comments
Closed
3 tasks done

Add advisory locking for POSIX based systems #123

ohsayan opened this issue Apr 16, 2021 · 2 comments
Assignees
@ohsayan
Labels
C-enhancement New feature or request C-reliability This issue/PR relates to reliability C-storage Relating to storage D-server Related to the server required-improvement This issue or PR addresses an already existing implementation that needs to be revised tracking-issue This is a tracking issue (to be used by maintainers only)

Comments

@ohsayan
Copy link
Member

ohsayan commented Apr 16, 2021 •
edited

The most important file for us during the lifecycle of the database server is the data.bin file that holds the serialized equivalent of the data in the in-memory table. Whenever BGSAVE kicks in, it writes to this file and then on the next run, it writes again and this goes on until the daemon is killed. However, we aren't doing something that we're supposed to ➡️ setting a lock on the file.

This issue aims to have this implemented for Unix based systems and not Windows systems just yet (this will be achieved in a future PR/commit). We'll be using locking as per the advisory record locking scheme in POSIX.

Tasks:

  • Basic implementation
  • Document
  • Tests

Related discussions: #14
@ohsayan ohsayan added C-enhancement New feature or request required-improvement This issue or PR addresses an already existing implementation that needs to be revised D-server Related to the server tracking-issue This is a tracking issue (to be used by maintainers only) C-storage Relating to storage C-reliability This issue/PR relates to reliability labels Apr 16, 2021
@ohsayan ohsayan self-assigned this Apr 16, 2021
ohsayan added a commit that referenced this issue Apr 16, 2021
@ohsayan
Add advisory locking for POSIX systems
a971058 
This commit adds a basic implementation of POSIX advisory record locking
which sets a lock on the `data.bin` file when the database server starts
and releases the lock when it terminates. This is just done for
compliance to let other processes know that we don't want them to use
the file.

However, the result depends entirely on the process that wants to do
'something' with the file. It is the responsibility of the process to
ensure that it respects the file lock.

Also, exclusive locks aren't perfect on Linux, so we can't rely on them.
See discussion #123 for more information.
@ohsayan ohsayan mentioned this issue Apr 16, 2021
Closed
@ohsayan
Copy link
Member Author

ohsayan commented Apr 17, 2021

The fcntl and flock controversy has just struck. This will need some work

@ohsayan
Copy link
Member Author

ohsayan commented Apr 17, 2021

Nope, we're using BSD locks. This will be addressed in a future issue

@ohsayan ohsayan closed this as completed Apr 17, 2021
@ohsayan ohsayan mentioned this issue Apr 18, 2021
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ohsayan ohsayan

Labels
C-enhancement New feature or request C-reliability This issue/PR relates to reliability C-storage Relating to storage D-server Related to the server required-improvement This issue or PR addresses an already existing implementation that needs to be revised tracking-issue This is a tracking issue (to be used by maintainers only)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add advisory locking for POSIX systems skytable/skytable
1 participant
@ohsayan