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

Minor improvements #203

Merged
merged 2 commits into from
Nov 11, 2020
Merged

Minor improvements #203

merged 2 commits into from
Nov 11, 2020

Conversation

gnodet
Copy link
Contributor

@gnodet gnodet commented Nov 11, 2020

No description provided.

@gnodet gnodet merged commit e40d2bb into apache:master Nov 11, 2020
@@ -232,7 +232,7 @@ public DaemonClientConnection startDaemon() {
return daemonConnection;
}
try {
sleep(200L);
sleep(10L);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to sleep here actually?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While the daemon is created, the registry does not contain any info, so the client would loop and read the registry continuously. But reading the registry involves complex file operations locking, so we need to let the daemon some space to actually write its own state to the registry.
It would be interesting to know how many iterations it takes on fast laptops.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ppalaga it's usually around 900ms / 1000ms to connect to a new daemon (the time between the start in this loop and the return statement), so it may be safer to revert to 200L to not consume too much cpu on not too fast computers.

@gnodet gnodet deleted the imp branch November 12, 2020 10:48
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.

None yet

2 participants