Skip to content

OAK-12181 - add metric when repository lock could not be renewed#2850

Merged
smiroslav merged 4 commits into
trunkfrom
issue/OAK-12181
Apr 24, 2026
Merged

OAK-12181 - add metric when repository lock could not be renewed#2850
smiroslav merged 4 commits into
trunkfrom
issue/OAK-12181

Conversation

@smiroslav
Copy link
Copy Markdown
Contributor

@smiroslav smiroslav requested review from joerghoh and jsedding April 15, 2026 14:34
@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

@jsedding jsedding left a comment

Choose a reason for hiding this comment

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

Looks good @smiroslav. Just one question/suggestion. Up to you if it makes sense or not.


private static void attachRemoteStoreMonitor(RemoteStoreMonitor remoteStoreMonitor) {
private void attachRemoteStoreMonitor(RemoteStoreMonitor remoteStoreMonitor) {
this.remoteStoreMonitor.set(remoteStoreMonitor);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would it make sense to only allow RemoteStoreMonitor to be set once?

Suggested change
this.remoteStoreMonitor.set(remoteStoreMonitor);
this.remoteStoreMonitor.getAndUpdate(oldValue -> {
if (oldValue == null) {
return remoteStoreMonitor;
}
throw new IllegalStateException("attachRemoteStoreMonitor may only be called once");
});

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks @jsedding for the suggestion.
I am thinking since the method is public, theoretically it could be called more than once for the same persistence, not that I see how it would make sense. I am a bit reluctant to add that set-once guard since we do not document this behaviour in the interface.

}

private void attachRemoteStoreMonitor(RemoteStoreMonitor remoteStoreMonitor) {
this.remoteStoreMonitor.set(remoteStoreMonitor);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See comment for V8.

@smiroslav smiroslav merged commit 02f31ca into trunk Apr 24, 2026
4 checks passed
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.

2 participants