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

Presence - Heartbeat Interval Should Be Heartbeat / 2 - 1 #33

Open
huonderv opened this issue Feb 19, 2021 · 1 comment
Open

Presence - Heartbeat Interval Should Be Heartbeat / 2 - 1 #33

huonderv opened this issue Feb 19, 2021 · 1 comment
Assignees
Labels
status: accepted This issue is accepted as valid, but it's not being worked on yet. type: bug This issue reports a bug.

Comments

@huonderv
Copy link

According to the PubNub REST API | Long running heartbeat calls, the heartbeatInterval should "automatically" be set to heartbeat / 2 -1:

if the client is set for a HB of x, the HBI is automatically set to x / 2 - 1. This enables, by default, that under excellent to average-bad network scenarios, the client should be able to "ping" the Presence server between 1 and 2 times before its HB timeout limit.

I believe that this is currently not reflected in the dart SDK.

Subscribe

Currently, the heartbeatInterval can be set via the SubscribeKeysetExtension:

set heartbeatInterval(int value) => settings['#heartbeatInterval'] = value;

This heartbeatInterval is then directly used as heartbeat value when subscribing to a channel in SubscribeParams:

'heartbeat': keyset.heartbeatInterval.toString()

Shouldn't the heartbeat that is sent in this request be (heartbeatInterval + 1) * 2?

PresenceWidget

In the PresenceWidget, the same heartbeatInterval is used both for setting the periodic Timer

   timer = Timer.periodic(
        Duration(seconds: widget.heartbeatInterval), _sendHeartbeat);

and as hearbeat sent to the server

  Future<void> _sendHeartbeat(Timer timer) async {
    ...
    await _pubnub.announceHeartbeat(
      heartbeat: widget.heartbeatInterval, // <== here
      channels: channels,
      channelGroups: channelGroups,
      keyset: widget.keyset,
      using: widget.using,
    );
  }

Shouldn't also there the heartbeat rather be (heartbeatInterval + 1) * 2?

Because with the current implementation, we got a lot of timeout presence events when the internet connection was not perfect.

Suggestion

I would strongly suggest to introduce an additional presenceTimeout and automatically set it to (heartbeatInterval + 1) * 2. Or the other way around and set the heartbeatInterval based on presenceTimeout, as e.g. done in the JavaScript SDK:

https://github.com/pubnub/javascript/blob/499b14bf6e0802c6389e6038add9bfd37af45601/src/core/components/config.js#L263

  setPresenceTimeout(val: number): this {
    ...
    this.setHeartbeatInterval(this._presenceTimeout / 2 - 1);
    return this;
  }

https://github.com/pubnub/javascript/blob/403e14cac7da6820239749686251080be4760022/src/core/endpoints/subscribe.js#L56

  const params: Object = {
    heartbeat: config.getPresenceTimeout(),
  };
@huonderv huonderv changed the title Presence - Heartbeat Interval Should Be Presence Timeout / 2 - 1 Presence - Heartbeat Interval Should Be Heartbeat / 2 - 1 Feb 19, 2021
@are are self-assigned this Feb 19, 2021
@are are added status: accepted This issue is accepted as valid, but it's not being worked on yet. type: bug This issue reports a bug. labels Feb 19, 2021
@are
Copy link
Contributor

are commented Feb 19, 2021

Hi there! Thanks for reporting this issue. You are absolutely right! We will include this fix in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepted This issue is accepted as valid, but it's not being worked on yet. type: bug This issue reports a bug.
Projects
None yet
Development

No branches or pull requests

2 participants