Skip to content

cli: make locking timeout configurable#363

Merged
jfontan merged 3 commits intosrc-d:masterfrom
jfontan:improvement/add-locking-timeout
Oct 17, 2018
Merged

cli: make locking timeout configurable#363
jfontan merged 3 commits intosrc-d:masterfrom
jfontan:improvement/add-locking-timeout

Conversation

@jfontan
Copy link
Contributor

@jfontan jfontan commented Oct 17, 2018

No description provided.

Signed-off-by: Javi Fontan <jfontan@gmail.com>
@jfontan jfontan requested review from a team and smola October 17, 2018 11:03
Copy link
Contributor

@smola smola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we test this?

Locking string `long:"locking" env:"BORGES_LOCKING" default:"local:" description:"locking service configuration"`
LockingTimeout string `long:"locking-timeout" env:"BORGES_LOCKING_TIMEOUT" default:"0" description:"timeout to acquire lock, 0 means no timeout"`
Workers int `long:"workers" env:"BORGES_WORKERS" default:"1" description:"number of workers, 0 means the same number as processors"`
Timeout string `long:"timeout" env:"BORGES_TIMEOUT" default:"10h" description:"deadline to process a job"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10h - wow!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some repos can take a lot of time. We use smaller values for our deployment.

Workers int `long:"workers" env:"BORGES_WORKERS" default:"1" description:"number of workers, 0 means the same number as processors"`
Timeout string `long:"timeout" env:"BORGES_TIMEOUT" default:"10h" description:"deadline to process a job"`
Locking string `long:"locking" env:"BORGES_LOCKING" default:"local:" description:"locking service configuration"`
LockingTimeout string `long:"locking-timeout" env:"BORGES_LOCKING_TIMEOUT" default:"0" description:"timeout to acquire lock, 0 means no timeout"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good to say a unit (in seconds)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added explanation about units.

Signed-off-by: Javi Fontan <jfontan@gmail.com>
Signed-off-by: Javi Fontan <jfontan@gmail.com>
@jfontan
Copy link
Contributor Author

jfontan commented Oct 17, 2018

@smola added a test for locking timeout. Unfortunately the error message does not tell me why it is failing so I have to measure times to be sure that it's failing because that timeout.

@jfontan jfontan merged commit b1cd5b9 into src-d:master Oct 17, 2018
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.

6 participants