From 394c8b05bb2960bdf5491e5776c8bdf220f09fc3 Mon Sep 17 00:00:00 2001 From: Dusan Malusev Date: Wed, 5 Aug 2026 11:02:21 +0200 Subject: [PATCH] fix(Cluster): pass seconds, not milliseconds, for heartbeat and TCP keepalive withConnectionHeartbeatInterval and withTCPKeepalive shared the seconds-to-milliseconds helper used by the timeout setters. The two cpp-driver functions behind them take seconds: cass_cluster_set_connection_heartbeat_interval(cluster, interval_secs) cass_cluster_set_tcp_keepalive(cluster, enabled, delay_secs) The result was a 1000x error. withConnectionHeartbeatInterval(30.0) sent 30000 seconds, about 8.3 hours, which disabled heartbeats. The defaults in php_scylladb_cluster_builder_new are raw seconds, so seconds was always the intended unit. Add php_scylladb_set_interval_seconds for the two settings that need seconds, and store the TCP keepalive delay in seconds. get_properties no longer divides tcpKeepalive by 1000, because the field is now seconds. The public PHP API does not change. Both methods still take seconds. Add tests/Unit/Cluster/BuilderTest.php. It checks the defaults, the seconds round-trip through the exposed connectionHeartbeatInterval and tcpKeepalive properties, the null case that disables keepalive, and the rejection of negative values. reconnectInterval is included as a control, because it stays in milliseconds internally. --- src/Cluster/Builder.c | 25 +++++++++++- src/Cluster/BuilderHandlers.c | 2 +- tests/Unit/Cluster/BuilderTest.php | 63 ++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 3 deletions(-) create mode 100644 tests/Unit/Cluster/BuilderTest.php diff --git a/src/Cluster/Builder.c b/src/Cluster/Builder.c index 7750f555b..acfa5c7cd 100644 --- a/src/Cluster/Builder.c +++ b/src/Cluster/Builder.c @@ -106,6 +106,27 @@ static zend_always_inline void php_scylladb_set_timeout(INTERNAL_FUNCTION_PARAME RETURN_ZVAL(getThis(), 1, 0); } +static zend_always_inline void php_scylladb_set_interval_seconds(INTERNAL_FUNCTION_PARAMETERS, uint32_t *out_seconds) +{ + double interval = 0; + + ZEND_PARSE_PARAMETERS_START(1, 1) + Z_PARAM_DOUBLE(interval) + ZEND_PARSE_PARAMETERS_END(); + + if (interval < 0) + { + zval val; + ZVAL_DOUBLE(&val, interval); + throw_invalid_argument(&val, "interval", "a positive number"); + return; + } + + *out_seconds = (uint32_t)ceil(interval); + + RETURN_ZVAL(getThis(), 1, 0); +} + ZEND_METHOD(Cassandra_Cluster_Builder, build) { #ifndef PHP_SCYLLADB_BACKEND_SCYLLA_RUST @@ -668,7 +689,7 @@ ZEND_METHOD(Cassandra_Cluster_Builder, withTCPKeepalive) } self->enable_tcp_keepalive = cass_true; - self->tcp_keepalive_delay = (uint32_t)ceil(delay * 1000); + self->tcp_keepalive_delay = (uint32_t)ceil(delay); RETURN_ZVAL(getThis(), 1, 0); } @@ -760,5 +781,5 @@ ZEND_METHOD(Cassandra_Cluster_Builder, withRandomizedContactPoints) ZEND_METHOD(Cassandra_Cluster_Builder, withConnectionHeartbeatInterval) { auto self = PHP_SCYLLADB_GET_CLUSTER_BUILDER(getThis()); - php_scylladb_set_timeout(INTERNAL_FUNCTION_PARAM_PASSTHRU, &self->connection_heartbeat_interval); + php_scylladb_set_interval_seconds(INTERNAL_FUNCTION_PARAM_PASSTHRU, &self->connection_heartbeat_interval); } diff --git a/src/Cluster/BuilderHandlers.c b/src/Cluster/BuilderHandlers.c index 2cda1c609..b405d1231 100644 --- a/src/Cluster/BuilderHandlers.c +++ b/src/Cluster/BuilderHandlers.c @@ -125,7 +125,7 @@ HashTable *php_scylladb_cluster_builder_properties(zend_object *object) if (self->enable_tcp_keepalive) { - ZVAL_DOUBLE(&tcpKeepalive, (double)self->tcp_keepalive_delay / 1000); + ZVAL_DOUBLE(&tcpKeepalive, (double)self->tcp_keepalive_delay); } else { diff --git a/tests/Unit/Cluster/BuilderTest.php b/tests/Unit/Cluster/BuilderTest.php new file mode 100644 index 000000000..b43838d51 --- /dev/null +++ b/tests/Unit/Cluster/BuilderTest.php @@ -0,0 +1,63 @@ +toBe(30); + }); + + it('defaults tcpKeepalive to null', function () { + $props = (array) Cassandra::cluster(); + + expect($props['tcpKeepalive'])->toBeNull(); + }); + + it('round-trips connectionHeartbeatInterval in seconds', function (float $interval, int $expected) { + $props = (array) Cassandra::cluster()->withConnectionHeartbeatInterval($interval); + + expect($props['connectionHeartbeatInterval'])->toBe($expected); + })->with([ + [0.0, 0], + [5.0, 5], + [30.0, 30], + [3600.0, 3600], + ]); + + it('round-trips tcpKeepalive in seconds', function (float $delay, float $expected) { + $props = (array) Cassandra::cluster()->withTCPKeepalive($delay); + + expect($props['tcpKeepalive'])->toBe($expected); + })->with([ + [0.0, 0.0], + [60.0, 60.0], + [1800.0, 1800.0], + ]); + + it('disables tcpKeepalive when given null', function () { + $props = (array) Cassandra::cluster()->withTCPKeepalive(null); + + expect($props['tcpKeepalive'])->toBeNull(); + }); + + it('round-trips reconnectInterval in seconds', function () { + $props = (array) Cassandra::cluster()->withReconnectInterval(2.5); + + expect($props['reconnectInterval'])->toBe(2.5); + }); + + it('rejects a negative connection heartbeat interval', function () { + Cassandra::cluster()->withConnectionHeartbeatInterval(-1.0); + })->throws(\Cassandra\Exception\InvalidArgumentException::class); + + it('rejects a negative tcp keepalive delay', function () { + Cassandra::cluster()->withTCPKeepalive(-1.0); + })->throws(\Cassandra\Exception\InvalidArgumentException::class); +});