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

Minor improvements #203

Merged
merged 2 commits into from
Nov 11, 2020
Merged

Minor improvements #203

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9357eee
Do not pass the maxKeepAlive as a parameter
gnodet Nov 10, 2020
2b7dd36
Reduce the sleep time
gnodet Nov 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions client/src/main/java/org/jboss/fuse/mvnd/client/DaemonClientConnection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,18 @@ public class DaemonClientConnection implements Closeable {
private final boolean newDaemon;
private boolean hasReceived;
private final Lock dispatchLock = new ReentrantLock();
private final int maxKeepAliveMs;
private final BlockingQueue<Message> queue = new ArrayBlockingQueue<>(16);
private final Thread receiver;
private final AtomicBoolean running = new AtomicBoolean(true);
private final AtomicReference<Exception> exception = new AtomicReference<>();
private final DaemonParameters parameters;

public DaemonClientConnection(DaemonConnection connection, DaemonInfo daemon,
StaleAddressDetector staleAddressDetector, boolean newDaemon, int maxKeepAliveMs, DaemonParameters parameters) {
StaleAddressDetector staleAddressDetector, boolean newDaemon, DaemonParameters parameters) {
this.connection = connection;
this.daemon = daemon;
this.staleAddressDetector = staleAddressDetector;
this.newDaemon = newDaemon;
this.maxKeepAliveMs = maxKeepAliveMs;
this.receiver = new Thread(this::doReceive);
this.receiver.start();
this.parameters = parameters;
Expand Down Expand Up @@ -96,6 +94,7 @@ public void dispatch(Message message) throws DaemonException.ConnectException {
}

public List<Message> receive() throws ConnectException, StaleAddressException {
int maxKeepAliveMs = parameters.keepAliveMs() * parameters.maxLostKeepAlive();
while (true) {
try {
final Message m = queue.poll(maxKeepAliveMs, TimeUnit.MILLISECONDS);
Expand Down
5 changes: 2 additions & 3 deletions client/src/main/java/org/jboss/fuse/mvnd/client/DaemonConnector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

} catch (InterruptedException e) {
throw new DaemonException.InterruptedException(e);
}
Expand Down Expand Up @@ -326,9 +326,8 @@ private DaemonClientConnection connectToDaemon(DaemonInfo daemon,
throws DaemonException.ConnectException {
LOGGER.debug("Connecting to Daemon");
try {
int maxKeepAliveMs = parameters.keepAliveMs() * parameters.maxLostKeepAlive();
DaemonConnection connection = connect(daemon.getAddress());
return new DaemonClientConnection(connection, daemon, staleAddressDetector, newDaemon, maxKeepAliveMs, parameters);
return new DaemonClientConnection(connection, daemon, staleAddressDetector, newDaemon, parameters);
} catch (DaemonException.ConnectException e) {
staleAddressDetector.maybeStaleAddress(e);
throw e;
Expand Down