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

Add options for lock statement timeout #183

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arthurprs
Copy link

This change was needed to get pg_repack to get the locks in our reasonably busy servers.

@schmiddy
Copy link
Member

schmiddy commented Jun 6, 2018

Thanks, I think a configurable lock_wait_max is useful. I'm less enthused about adding yet another option for --lock-wait-step, this seems like something we should be able to guesstimate on our own, without contributing to the explosion of command-line options.

What if we just had the lock wait step be 10% of the lock_wait_max, with some minimum like 100 ms.

@arthurprs
Copy link
Author

I don't have strong opinions but I'd suggest keeping the step parameter as it's strictly more useful (arguably to a small group but it doesn't matter to the other).

Defaulting wait-step to 10% of max-wait sounds nice a nice default though.

@arthurprs
Copy link
Author

I finally had the time to revisit this. Change updated as you suggested.

@andreasscherman
Copy link

Thanks for the pull request @arthurprs! This is exactly what I was looking for --- we have a highly contested server and was in need of having a variable timeout for the lock.

@schmiddy would you mind merging this?

@hstefan
Copy link

hstefan commented Nov 14, 2018

We have a similar patch internally, and would greatly appreciate having it merged so that we can use the upstream package.

@andreasscherbaum
Copy link
Collaborator

@arthurprs Can you please rebase this PR? This will trigger the Checks.
Also the documentation is not updated with the new flag.

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

Successfully merging this pull request may close these issues.

6 participants