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

JAVA: keep alive not working #117

Open
konradmichael opened this issue Jan 15, 2019 · 4 comments
Open

JAVA: keep alive not working #117

konradmichael opened this issue Jan 15, 2019 · 4 comments

Comments

@konradmichael
Copy link
Contributor

konradmichael commented Jan 15, 2019

The keep alive does not seem to work, maybe i also found out why it is so.

Reproduce:

  1. Connect to any remote mqtt-broker (not on your pc).
  2. unplug network cable
    Result:
    Nothing. The connection will not be closed

If you replug the network, the connection will be closed and reconnected (in my setup).

Potential error: MqttClientImpl line 140

netClientOptions.setIdleTimeout(DEFAULT_IDLE_TIMEOUT);

Context:

public MqttClientImpl(Vertx vertx, MqttClientOptions options) {

    // copy given options
    NetClientOptions netClientOptions = new NetClientOptions(options);
    netClientOptions.setIdleTimeout(DEFAULT_IDLE_TIMEOUT);

    this.client = vertx.createNetClient(netClientOptions);
    this.options = options;
}

The DEFAULT_IDLE_TIMEOUT is 0. Zero disabled timeout detection,
I belive this should be keepAlive * 1.5

My current workaround is:

Future<Void> disconnectTask;
ScheduledExecutorService scheduledExecutorService;
int keepAlive = 30;
int timeout = 45;

// keep alive with multiplier to prevent multiple pings
mqttClientOptions.setKeepAliveTimeSeconds(keepAlive * 2);

mqttClient.pingResponseHandler(e ->  {
  // cancel timeout task
	disconnectTask.cancel(false);
  // schedule new timeout task
	disconnectTask = scheduledExecutorService.schedule(() -> {
		logger.warn("disconnecting because keep alive fails!");
		client.disconnect();
		return null;
	}, timeout, TimeUnit.SECONDS);
	// keep alive will not be send if other messages are processed,
	// force sending or it will get complicated!
	scheduledExecutorService.schedule(() -> client.ping(), keepAlive, TimeUnit.SECONDS);
}

I would be glad to receive some feedback

@konradmichael
Copy link
Contributor Author

Pull request for working solution: #118

@vietj
Copy link
Contributor

vietj commented Jan 17, 2019

can you elaborate what does the 1.5 factor does ? it is not clear to me what is the actual issue and what the fix changes.

@konradmichael
Copy link
Contributor Author

Issue: Client will not detect a missing ping response as a disconnect.
Fix: terminates connection if no response is comming from broker (timeout)

The use of the keepAlive-Message is to detect if the connection to the broker is lost.
So if the keepAlive message is no longer responded by the broker, the connection is dead.

Currently you will not be informed if the connection to your mqtt-broker is lost, status will still be "connected".
With the above workaround or my pull-request you will disconnect from the broker if the keep-alive is not send.

The Factor 1.5 is comming from the MQTT-(Spec), but from the broker side of it.

The broker must disconnect a client that does not send a a message or a PINGREQ packet in one and a half times the keep alive interval

Source: HiveMQ - keep alive

You can also use a factor of 2 or 3 as the timeout, but I wanted to stay close to the spec.

@konradmichael
Copy link
Contributor Author

Update:

The DEFAULT_IDLE_TIMEOUT is 0. Zero disabled timeout detection,
I belive this should be keepAlive * 1.5

This is not working, I tried it, see the pull-request for a working solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants