Skip to content

Do heartbeat and leave calls in sequence#237

Merged
kleewho merged 7 commits intomasterfrom
CLEN-397
May 9, 2022
Merged

Do heartbeat and leave calls in sequence#237
kleewho merged 7 commits intomasterfrom
CLEN-397

Conversation

@kleewho
Copy link
Copy Markdown
Contributor

@kleewho kleewho commented Apr 28, 2022

fix: Do heartbeat and leave calls sequentially

It prevents race condition between these two calls and "phantom" join after leave

In some circumstances we do two contradicting calls in almost
the same time. One of them is a call to heartbeat endpoint and
the other is to leave endpoint. The reason why those call could
happen in almost the same time is because heartbeat calls are done
periodically on separate thread and leave calls are happening due to
user actions. This means that sooner or later this situation will happen
and pubnub presence system will generate join event and leave event (usually
in this order), but then another timeout event when presence timeout
expires.

To prevent it from happening I've decided to limit the number of concurrent
calls to only those two endpoints to just one. This is done on the okhttp
client level. Because those two calls can be ordered only two ways we can
provide an analysis of why this change might work.

Let's first concentrate on situation when heartbeat call is already happening
and unsubscribe method is being called:

* leave call will go to the internal queue of calls of okhttp client due to
limit of concurrent calls
* unsubscribe method internally will try to cancel the heartbeat call and schedule
timer with another heartbeat call (but with different set of channels)
* heartbeat finishes (either normally, or by being cancelled), this usually
do not generate any events, but it might generate join event
* http leave call can finally proceed, which generates leave event
* because the calls have been done sequentially there's no timeout event

In case when leave call is already ongoing and the schedule heartbeat timer
is trying to make heartbeat request:

* heartbeat request goes to client's internal queue to wait on leave to be finished
* in the meantime usubscribe method internally cancels the timer and cancels the heartbeat
request before it has time to start
* leave request ends, and leave event is being generated
* there's no timeout event, and the presence didn't have chance to be extended
this.signatureInterceptor = new SignatureInterceptor(pubNubInstance);

if (!pubNubInstance.getConfiguration().isGoogleAppEngineNetworking()) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code format

Copy link
Copy Markdown

@Chesteer89 Chesteer89 left a comment

Choose a reason for hiding this comment

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

LGTM! :)

<Class name="com.pubnub.api.PubNubException"/>
<Field name="affectedCall"/>
<Bug pattern="URF_UNREAD_FIELD"/>
</Match>
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.

Don't know why it started to be a problem now. I didn't touch it at all yet here it is

@kleewho
Copy link
Copy Markdown
Contributor Author

kleewho commented May 9, 2022

@client-engineering-bot release as v6.0.1

Copy link
Copy Markdown
Contributor

@parfeon parfeon left a comment

Choose a reason for hiding this comment

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

@kleewho don't forget to make same changes on GitHub release.

@kleewho kleewho merged commit bfcae43 into master May 9, 2022
@kleewho kleewho deleted the CLEN-397 branch May 9, 2022 15:10
@github-actions
Copy link
Copy Markdown

github-actions bot commented May 9, 2022

🚀 Release successfully completed 🚀

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.

4 participants