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

compact: open src file readonly #292

Merged
merged 1 commit into from
Jan 29, 2022

Conversation

missinglink
Copy link
Contributor

@missinglink missinglink commented Oct 2, 2021

the compact CLI command opens the source file as 0444 but doesn't currently set ReadOnly: true.

it therefore requests a read-write file lock on the source database when it only really needs read-only.

this is due to the flock code only considering the db.readOnly bool and not the os.FileMode.

the problem is that if one or more processes have read-only locks then this operation will stall waiting to acquire a write lock that it doesn't need 🤷‍♂️

with this PR the ReadOnly: true option is set explicitly, meaning that it will play nice with other processes which are also read-only.

@ptabor
Copy link
Contributor

ptabor commented Oct 18, 2021

Thank you. Looks good to me.

@missinglink
Copy link
Contributor Author

Is there still something preventing this being merged?

@xiang90 xiang90 merged commit 9b8a60d into etcd-io:master Jan 29, 2022
@xiang90
Copy link
Contributor

xiang90 commented Jan 29, 2022

@missinglink No. Merged.

@jdbPAC2387
Copy link

@missinglink, you are the man/etc.~'pc'

@tmm1
Copy link
Contributor

tmm1 commented Oct 26, 2022

ReadOnly is broken on Windows without #307

@ahrtr ahrtr added this to the 1.3.7 milestone Dec 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants