Skip to content

Send a real EventSource event for heartbeat#222

Closed
gdamjan wants to merge 1 commit intoapache:masterfrom
gdamjan:master
Closed

Send a real EventSource event for heartbeat#222
gdamjan wants to merge 1 commit intoapache:masterfrom
gdamjan:master

Conversation

@gdamjan
Copy link
Copy Markdown
Contributor

@gdamjan gdamjan commented May 3, 2014

The EventSource connection can get stuck (in TCP half-open state*) and there's no way
for the client to detect that. This commit changes the way heartbeat is sent, instead of
sending a newline character, it sends an empty event of type heartbeat:

event: heartbeat
data:

This event doesn't have an id: field, so the client will retain its latest Last-Event-ID state.

This doesn't change the expectations of clients that used EventSource till now, because they
subscribe to the 'message' event type. To get the 'heartbeat' events a client will need to
explicitly subscribe to it:

source.addEventListener('heartbeat', function () { /* cancel a timer that would otherwise reconnect the source */ });
  • this can happen when you suspend your laptop, on flaky internet connection, ADSL reconnect,
    bad wifi signals, bad routers etc. Pretty often in a typical internet usage nowadays.

@kxepal
Copy link
Copy Markdown
Member

kxepal commented May 3, 2014

+1

@gdamjan
Copy link
Copy Markdown
Contributor Author

gdamjan commented May 4, 2014

I'm have doubts about the test, maybe it should check for heartbeat_count > 3 or something ?
also, will "make check" even run that? I don't see the link for browser tests in master

@kxepal
Copy link
Copy Markdown
Member

kxepal commented May 4, 2014

@gdamjan with !!window.EventSource check it wouldn't be tested with make check since there is no any window (iirc, but you may always make sure about by making your test explicitly and always failed), but you may navigate to browser tests by direct url:
http://localhost:5984/_utils/couch_tests.html
While like was removed from sidebar, the page and test running suite is still available.

@gdamjan
Copy link
Copy Markdown
Contributor Author

gdamjan commented May 4, 2014

ok, thanks.
changes test passed in browser.

@KlausTrainer
Copy link
Copy Markdown
Contributor

@gdamjan Aside from the typo it looks good overall. Would you mind fixing it so I can commit it?

The EventSource connection can get stuck (in TCP half-open state*) and there's no way
for the client to detect that. This commit changes the way heartbeat is sent, instead of
sending a newline character, it sends an empty event of type heartbeat:

    event: heartbeat
    data:

This event doesn't have an id: field, so the client will retain its latest Last-Event-ID state.

This doesn't change the expectations of clients that used EventSource till now, because they
subscribe to the 'message' event type. To get the 'heartbeat' events a client will need to
explicitly subscribe to it:

    source.addEventListener('heartbeat', function () { /* cancel a timer that would otherwise reconnect the source */ });

* this can happen when you suspend your laptop, on flaky internet connection, ADSL reconnect,
bad wifi signals, bad routers etc. Pretty often in a typical internet usage nowadays.
@gdamjan
Copy link
Copy Markdown
Contributor Author

gdamjan commented May 13, 2014

@gdamjan gdamjan closed this May 13, 2014
janl pushed a commit that referenced this pull request Jan 5, 2020
The link to couch_replicator_utils.erl is broken and should point to github code. 

In the new code, couch_replicator_utils.erl does not contains the ID generation logic so the reference is update to point to couch_replicator_ids.erl where de ID is build.

Resolves apache/couchdb-documentation#221
nickva pushed a commit to nickva/couchdb that referenced this pull request Sep 7, 2022
The link to couch_replicator_utils.erl is broken and should point to github code. 

In the new code, couch_replicator_utils.erl does not contains the ID generation logic so the reference is update to point to couch_replicator_ids.erl where de ID is build.

Resolves apache/couchdb-documentation#221
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.

3 participants