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

Reintroduce advisory locking when starting the node #5215

Merged
merged 2 commits into from
Oct 18, 2021
Merged

Reintroduce advisory locking when starting the node #5215

merged 2 commits into from
Oct 18, 2021

Conversation

archseer
Copy link
Contributor

No description provided.

@github-actions
Copy link
Contributor

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

3 similar comments
@github-actions
Copy link
Contributor

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

@github-actions
Copy link
Contributor

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

@github-actions
Copy link
Contributor

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

@github-actions
Copy link
Contributor

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

@github-actions
Copy link
Contributor

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

@archseer archseer marked this pull request as ready for review October 13, 2021 06:05
jmank88
jmank88 reviewed Oct 13, 2021
View reviewed changes
core/services/chainlink/application.go Outdated
}

// Try to immediately acquire an advisory lock. The lock will be released on application stop.
func (app *ChainlinkApplication) AdvisoryLock(ctx context.Context, timeout time.Duration) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be a little cleaner to pass just one of these two params?

Copy link
Contributor Author

@archseer archseer Oct 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

It's not quite the same: the context times out the entire operation, but the timeout is for how long before timing out each lock attempt inside the loop.

We're hoping to remove this lock altogether anyway, I had to re-add it because I removed the old implementation in the store and some operators are relying on this to run a fallback node (spin up two nodes, have one keep looping, if primary node dies it takes over)

samsondav
samsondav reviewed Oct 15, 2021
View reviewed changes
core/cmd/local_client.go Outdated Show resolved Hide resolved
samsondav
samsondav previously approved these changes Oct 15, 2021
View reviewed changes
samsondav
samsondav previously approved these changes Oct 16, 2021
View reviewed changes
@samsondav samsondav merged commit ee26835 into develop Oct 18, 2021
@samsondav samsondav deleted the advisory-locks branch October 18, 2021 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samsondav samsondav samsondav approved these changes

@jkongie jkongie Awaiting requested review from jkongie

@jmank88 jmank88 Awaiting requested review from jmank88

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@archseer @jmank88 @samsondav