Page MenuHomePhabricator

[Chronik] Send regular pings from WebSocket to prevent disconnects
ClosedPublic

Authored by tobias_ruck on Jan 3 2024, 15:31.

Details

Summary

This sends regular Ping messages from the websocket to keep the connection alive.

From empicial testing, it seems like 45s is a good number to keep the connection alive for both browser and NodeJS WebSocket connections.

We have to add a ChronikSettings to tune WS connection, because so far all configs were for the indexer only, but not for the HTTP/WS side of Chronik.

Test Plan

./test/functional/test_runner.py chronik_ws_ping

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Tail of the build log:

> tsc


added 258 packages, and audited 259 packages in 7s

48 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

> chronik-client@0.10.0 integration-tests
> mocha -j1 -r ts-node/register test/integration/*.ts --reporter mocha-junit-reporter --reporter-options mochaFile=test_results/chronik-client-integration-tests-junit.xml --reporter-options testsuitesTitle=Chronik Client Integration Tests --reporter-options rootSuiteTitle=Chronik Client

Starting test_runner
Test runner started
2024-01-03T15:56:02.515000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/chronik-client-integration-tests/test/tmp/test_runner_โ‚ฟโ‚ต_๐Ÿƒ_20240103_155602/setup_scripts/chronik-client_blockchain_info_0
Setting chronik url to  http://127.0.0.1:30001
2024-01-03T15:56:04.469000Z TestFramework (INFO): Step 1: Mine 10 more blocks
2024-01-03T15:56:04.478000Z TestFramework (INFO): Received a stop message, exiting
2024-01-03T15:56:04.529000Z TestFramework (INFO): Stopping nodes
2024-01-03T15:56:04.982000Z TestFramework (INFO): Cleaning up /work/abc-ci-builds/chronik-client-integration-tests/test/tmp/test_runner_โ‚ฟโ‚ต_๐Ÿƒ_20240103_155602/setup_scripts/chronik-client_blockchain_info_0 on exit
2024-01-03T15:56:04.982000Z TestFramework (INFO): Tests successful
Running Unit Tests for Test Framework Modules
setup_scripts/chronik-client_blockchain_info.py started
setup_scripts/chronik-client_blockchain_info.py passed, Duration: 3 s

TEST                                            | STATUS    | DURATION

setup_scripts/chronik-client_blockchain_info.py | โœ“ Passed  | 3 s

ALL                                             | โœ“ Passed  | 3 s (accumulated) 
Runtime: 3 s

-----------------------|---------|----------|---------|---------|-----------------------------------
File                   | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                 
-----------------------|---------|----------|---------|---------|-----------------------------------
All files              |    5.36 |     0.63 |    1.47 |    5.34 |                                   
 chronik-client        |     100 |      100 |     100 |     100 |                                   
  index.ts             |     100 |      100 |     100 |     100 |                                   
 chronik-client/proto  |     4.9 |     0.67 |    1.72 |    4.89 |                                   
  chronik.ts           |    5.44 |     0.84 |    1.81 |    5.42 | ...,3978-3985,3990-4027,4031-4036 
  chronikNode.ts       |    4.15 |     0.45 |    1.59 |    4.17 | ...,2682-2729,2740-2804,2808-2813 
 chronik-client/src    |    8.51 |        0 |       0 |    8.46 |                                   
  ChronikClient.ts     |    4.19 |        0 |       0 |    4.24 | 29-159,174-218,296-699            
  ChronikClientNode.ts |      40 |      100 |       0 |      40 | 23-42                             
  failoverProxy.ts     |    3.92 |        0 |       0 |    4.04 | 27-311                            
  hex.ts               |   31.57 |        0 |       0 |   33.33 | 30-34,38-42,46-59,63-65           
-----------------------|---------|----------|---------|---------|-----------------------------------

##teamcity[blockOpened name='Code Coverage Summary']
##teamcity[buildStatisticValue key='CodeCoverageAbsBCovered' value='156']
##teamcity[buildStatisticValue key='CodeCoverageAbsBTotal' value='2909']
##teamcity[buildStatisticValue key='CodeCoverageAbsRCovered' value='21']
##teamcity[buildStatisticValue key='CodeCoverageAbsRTotal' value='3286']
##teamcity[buildStatisticValue key='CodeCoverageAbsMCovered' value='8']
##teamcity[buildStatisticValue key='CodeCoverageAbsMTotal' value='541']
##teamcity[buildStatisticValue key='CodeCoverageAbsLCovered' value='154']
##teamcity[buildStatisticValue key='CodeCoverageAbsLTotal' value='2881']
##teamcity[blockClosed name='Code Coverage Summary']
Build chronik-client-integration-tests failed with exit code 1

I don't know what is causing this failure, something returns non zero despite everything seems to succeed...

@bot chronik-client-integration-tests

Fabien requested changes to this revision.Jan 3 2024, 17:45

Can we make the interval much shorter on regtest ? It's sad the test has to last > 45s for no reason

chronik/chronik-http/src/ws.rs
108 โ†—(On Diff #43853)

why do you fill the ping message with garbage?

This revision now requires changes to proceed.Jan 3 2024, 17:45

If we reduce the ping timer on regtest, shall we also remove the pings coming from the test framework?

If we reduce the ping timer on regtest, shall we also remove the pings coming from the test framework?

Even if I don't think it hurts, it's probably no longer necessary. We can still use it to detect disconnects but that's unlikely to be useful in the test framework

reduce ping interval on regtest to 5s, make ping a readable message

@Fabien Agreed, but I think it's better to remove it in a separate diff.

@Fabien Agreed, but I think it's better to remove it in a separate diff.

Yes

Fabien requested changes to this revision.Jan 4 2024, 20:58
Fabien added inline comments.
chronik/chronik-cpp/chronik.cpp
23 โ†—(On Diff #43897)

With #include<chrono>

26 โ†—(On Diff #43897)

Dito

54 โ†—(On Diff #43897)

Iirc there is a count_seconds() function somewhere, likely in util/time.h

test/functional/chronik_ws_ping.py
49 โ†—(On Diff #43897)

Better use the wait_until facility

This revision now requires changes to proceed.Jan 4 2024, 20:58

use std::chrono::seconds, use wait_until, update copyright year

Fabien added inline comments.
chronik/chronik-cpp/chronik.cpp
57โ€“58 โ†—(On Diff #43904)

Nit

This revision is now accepted and ready to land.Jan 5 2024, 07:40