Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ modules/valkey_glide.so
libtool
cluster_scan_cursor_legacy_arginfo.h
cluster_scan_cursor_arginfo.h
tests/valkey_data/*/valkey.log
tests/valkey_data/
composer.lock
configure
configure.ac
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Changes

* Add circuit breaker configuration support for standalone and cluster clients ([#238](https://github.com/valkey-io/valkey-glide-php/issues/238))
* Add `MIGRATE` command for standalone and cluster clients ([#276](https://github.com/valkey-io/valkey-glide-php/pull/276))
* Add `CLIENT PAUSE` and `CLIENT UNPAUSE` commands for standalone and cluster clients ([#274](https://github.com/valkey-io/valkey-glide-php/pull/274))
* Add `RESET` command for standalone and cluster clients ([#273](https://github.com/valkey-io/valkey-glide-php/pull/273))
Expand Down
28 changes: 28 additions & 0 deletions common.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,31 @@ typedef struct {
bool has_max_decompressed_size; /* true if user explicitly set max_decompressed_size */
} valkey_glide_compression_config_t;

/* Circuit breaker configuration */
#define VALKEY_GLIDE_CB_WINDOW_SIZE_MS "window_size_ms"
#define VALKEY_GLIDE_CB_FAILURE_RATE_THRESHOLD "failure_rate_threshold"
#define VALKEY_GLIDE_CB_MIN_ERRORS "min_errors"
#define VALKEY_GLIDE_CB_OPEN_TIMEOUT_MS "open_timeout_ms"
#define VALKEY_GLIDE_CB_COUNT_TIMEOUTS "count_timeouts"
#define VALKEY_GLIDE_CB_CONSECUTIVE_SUCCESSES "consecutive_successes"

#define VALKEY_GLIDE_CB_DEFAULT_WINDOW_SIZE_MS 10000
#define VALKEY_GLIDE_CB_DEFAULT_FAILURE_RATE_THRESHOLD 0.5
#define VALKEY_GLIDE_CB_DEFAULT_MIN_ERRORS 50
#define VALKEY_GLIDE_CB_DEFAULT_OPEN_TIMEOUT_MS 5000
#define VALKEY_GLIDE_CB_DEFAULT_COUNT_TIMEOUTS false
#define VALKEY_GLIDE_CB_DEFAULT_CONSECUTIVE_SUCCESSES 3
#define VALKEY_GLIDE_ERROR_TYPE_CIRCUIT_BREAKER_OPEN 4
Comment thread
currantw marked this conversation as resolved.

typedef struct {
uint32_t window_size_ms;
float failure_rate_threshold;
uint32_t min_errors;
uint32_t open_timeout_ms;
bool count_timeouts;
uint32_t consecutive_successes;
} valkey_glide_circuit_breaker_config_t;

/* Client-side cache configuration */
typedef struct {
char* cache_id; /* Unique cache identifier */
Expand Down Expand Up @@ -187,6 +212,7 @@ typedef struct {
valkey_glide_advanced_base_client_configuration_t* advanced_config; /* NULL if not set */
valkey_glide_compression_config_t* compression_config; /* NULL if not set */
valkey_glide_client_side_cache_config_t* client_side_cache; /* NULL if not set */
valkey_glide_circuit_breaker_config_t* circuit_breaker; /* NULL if not set */
valkey_glide_read_from_t read_from;
int addresses_count;
int request_timeout; /* -1 if not set */
Expand Down Expand Up @@ -222,6 +248,7 @@ typedef struct {
zval* context; /* Stream context for TLS */
zval* compression; /* Compression configuration */
zval* client_side_cache; /* Client-side cache configuration */
zval* circuit_breaker; /* Circuit breaker configuration */
char* client_name;
char* client_az;
size_t client_name_len;
Expand Down Expand Up @@ -318,6 +345,7 @@ PHP_MINFO_FUNCTION(redis);

zend_class_entry* get_valkey_glide_ce(void);
zend_class_entry* get_valkey_glide_exception_ce(void);
zend_class_entry* get_valkey_glide_circuit_breaker_exception_ce(void);

zend_class_entry* get_valkey_glide_cluster_ce(void);

Expand Down
6 changes: 4 additions & 2 deletions src/client_constructor_mock.c
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ PHP_METHOD(ClientConstructorMock, simulate_standalone_constructor) {
valkey_glide_php_common_constructor_params_t common_params;
valkey_glide_init_common_constructor_params(&common_params);

ZEND_PARSE_PARAMETERS_START(0, 15)
ZEND_PARSE_PARAMETERS_START(0, 16)
Z_PARAM_OPTIONAL
Z_PARAM_ARRAY_OR_NULL(common_params.addresses)
Z_PARAM_BOOL(common_params.use_tls)
Expand All @@ -110,6 +110,7 @@ PHP_METHOD(ClientConstructorMock, simulate_standalone_constructor) {
Z_PARAM_ARRAY_OR_NULL(common_params.compression)
Z_PARAM_ARRAY_OR_NULL(common_params.client_side_cache)
Z_PARAM_ZVAL_OR_NULL(common_params.address_resolver)
Z_PARAM_ARRAY_OR_NULL(common_params.circuit_breaker)
ZEND_PARSE_PARAMETERS_END_EX(RETURN_THROWS());

/* Validate database_id range before setting */
Expand Down Expand Up @@ -159,7 +160,7 @@ PHP_METHOD(ClientConstructorMock, simulate_cluster_constructor) {
valkey_glide_php_common_constructor_params_t common_params;
valkey_glide_init_common_constructor_params(&common_params);

ZEND_PARSE_PARAMETERS_START(0, 16)
ZEND_PARSE_PARAMETERS_START(0, 17)
Z_PARAM_OPTIONAL
Z_PARAM_ARRAY_OR_NULL(common_params.addresses)
Z_PARAM_BOOL(common_params.use_tls)
Expand All @@ -178,6 +179,7 @@ PHP_METHOD(ClientConstructorMock, simulate_cluster_constructor) {
Z_PARAM_ARRAY_OR_NULL(common_params.compression)
Z_PARAM_ARRAY_OR_NULL(common_params.client_side_cache)
Z_PARAM_ZVAL_OR_NULL(common_params.address_resolver)
Z_PARAM_ARRAY_OR_NULL(common_params.circuit_breaker)
ZEND_PARSE_PARAMETERS_END_EX(RETURN_THROWS());

/* Validate database_id range before setting */
Expand Down
2 changes: 2 additions & 0 deletions src/client_constructor_mock.stub.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ public static function simulate_standalone_constructor(
?array $compression = null,
?array $client_side_cache = null,
?callable $address_resolver = null,
?array $circuit_breaker = null,
): \Connection_request\ConnectionRequest;

/**
Expand Down Expand Up @@ -164,5 +165,6 @@ public static function simulate_cluster_constructor(
?array $compression = null,
?array $client_side_cache = null,
?callable $address_resolver = null,
?array $circuit_breaker = null,
): \Connection_request\ConnectionRequest;
}
91 changes: 91 additions & 0 deletions tests/ConnectionRequestTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
require_once __DIR__ . "/TestSuite.php";
require_once __DIR__ . "/../vendor/autoload.php";
require_once __DIR__ . "/Connection_request/AuthenticationInfo.php";
require_once __DIR__ . "/Connection_request/ClientCircuitBreakerConfig.php";
require_once __DIR__ . "/Connection_request/CompressionBackend.php";
require_once __DIR__ . "/Connection_request/CompressionConfig.php";
require_once __DIR__ . "/Connection_request/ConnectionRequest.php";
Expand Down Expand Up @@ -742,4 +743,94 @@ private function get_addresses(\Connection_request\ConnectionRequest $request):
{
return array_map(fn($a) => $a->getHost(), iterator_to_array($request->getAddresses()));
}

// ================================================================
// Circuit Breaker Tests
// ================================================================
Comment thread
prateek-kumar-improving marked this conversation as resolved.
Comment thread
currantw marked this conversation as resolved.

public function testCircuitBreakerStandaloneDefaults()
{
$request = ClientConstructorMock::simulate_standalone_constructor(
circuit_breaker: []
);

$cb = $request->getClientCircuitBreaker();
$this->assertNotNull($cb);
$this->assertEquals(10000, $cb->getWindowSizeMs());
$this->assertTrue(abs($cb->getFailureRateThreshold() - 0.5) < 0.001);
$this->assertEquals(50, $cb->getMinErrors());
$this->assertEquals(5000, $cb->getOpenTimeoutMs());
$this->assertFalse($cb->getCountTimeouts());
$this->assertEquals(3, $cb->getConsecutiveSuccesses());
}

public function testCircuitBreakerClusterDefaults()
{
$request = ClientConstructorMock::simulate_cluster_constructor(
circuit_breaker: []
);

$cb = $request->getClientCircuitBreaker();
$this->assertNotNull($cb);
$this->assertEquals(10000, $cb->getWindowSizeMs());
$this->assertTrue(abs($cb->getFailureRateThreshold() - 0.5) < 0.001);
$this->assertEquals(50, $cb->getMinErrors());
$this->assertEquals(5000, $cb->getOpenTimeoutMs());
$this->assertFalse($cb->getCountTimeouts());
$this->assertEquals(3, $cb->getConsecutiveSuccesses());
}

public function testCircuitBreakerStandaloneCustomValues()
{
$request = ClientConstructorMock::simulate_standalone_constructor(
circuit_breaker: [
'window_size_ms' => 20000,
'failure_rate_threshold' => 0.8,
'min_errors' => 10,
'open_timeout_ms' => 3000,
'count_timeouts' => true,
'consecutive_successes' => 5,
]
);

$cb = $request->getClientCircuitBreaker();
$this->assertNotNull($cb);
$this->assertEquals(20000, $cb->getWindowSizeMs());
$this->assertTrue(abs($cb->getFailureRateThreshold() - 0.8) < 0.001);
$this->assertEquals(10, $cb->getMinErrors());
$this->assertEquals(3000, $cb->getOpenTimeoutMs());
$this->assertTrue($cb->getCountTimeouts());
$this->assertEquals(5, $cb->getConsecutiveSuccesses());
}

public function testCircuitBreakerClusterCustomValues()
{
$request = ClientConstructorMock::simulate_cluster_constructor(
circuit_breaker: [
'window_size_ms' => 15000,
'failure_rate_threshold' => 0.3,
'min_errors' => 25,
'open_timeout_ms' => 10000,
'count_timeouts' => true,
'consecutive_successes' => 2,
]
);

$cb = $request->getClientCircuitBreaker();
$this->assertNotNull($cb);
$this->assertEquals(15000, $cb->getWindowSizeMs());
$this->assertTrue(abs($cb->getFailureRateThreshold() - 0.3) < 0.001);
$this->assertEquals(25, $cb->getMinErrors());
$this->assertEquals(10000, $cb->getOpenTimeoutMs());
$this->assertTrue($cb->getCountTimeouts());
$this->assertEquals(2, $cb->getConsecutiveSuccesses());
}

public function testCircuitBreakerNotSetByDefault()
{
$request = ClientConstructorMock::simulate_standalone_constructor();

$cb = $request->getClientCircuitBreaker();
$this->assertNull($cb);
}
}
84 changes: 81 additions & 3 deletions valkey_glide.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ static const size_t SSL_PREFIX_LEN = 6;

zend_class_entry* valkey_glide_ce;
zend_class_entry* valkey_glide_exception_ce;
zend_class_entry* valkey_glide_circuit_breaker_exception_ce;

zend_class_entry* valkey_glide_cluster_ce;

Expand All @@ -92,6 +93,10 @@ zend_class_entry* get_valkey_glide_exception_ce(void) {
return valkey_glide_exception_ce;
}

zend_class_entry* get_valkey_glide_circuit_breaker_exception_ce(void) {
return valkey_glide_circuit_breaker_exception_ce;
}

zend_class_entry* get_valkey_glide_cluster_ce(void) {
return valkey_glide_cluster_ce;
}
Expand Down Expand Up @@ -183,6 +188,7 @@ void valkey_glide_init_common_constructor_params(
params->context = NULL;
params->compression = NULL;
params->client_side_cache = NULL;
params->circuit_breaker = NULL;
params->address_resolver = NULL;
}

Expand Down Expand Up @@ -573,6 +579,60 @@ int valkey_glide_build_client_config_base(valkey_glide_php_common_constructor_pa
config->client_side_cache = NULL;
}

Comment thread
currantw marked this conversation as resolved.
/* Process circuit breaker configuration if provided */
if (params->circuit_breaker && Z_TYPE_P(params->circuit_breaker) == IS_ARRAY) {
HashTable* cb_ht = Z_ARRVAL_P(params->circuit_breaker);

config->circuit_breaker = ecalloc(1, sizeof(valkey_glide_circuit_breaker_config_t));

/* Parse window_size_ms (optional, default 10000) */
zval* window_zv = zend_hash_str_find(
cb_ht, VALKEY_GLIDE_CB_WINDOW_SIZE_MS, sizeof(VALKEY_GLIDE_CB_WINDOW_SIZE_MS) - 1);
config->circuit_breaker->window_size_ms = window_zv
? (uint32_t) zval_get_long(window_zv)
: VALKEY_GLIDE_CB_DEFAULT_WINDOW_SIZE_MS;

/* Parse failure_rate_threshold (optional, default 0.5) */
zval* threshold_zv = zend_hash_str_find(cb_ht,
VALKEY_GLIDE_CB_FAILURE_RATE_THRESHOLD,
sizeof(VALKEY_GLIDE_CB_FAILURE_RATE_THRESHOLD) - 1);
config->circuit_breaker->failure_rate_threshold =
threshold_zv ? (float) zval_get_double(threshold_zv)
: VALKEY_GLIDE_CB_DEFAULT_FAILURE_RATE_THRESHOLD;

/* Parse min_errors (optional, default 50) */
zval* min_errors_zv = zend_hash_str_find(
cb_ht, VALKEY_GLIDE_CB_MIN_ERRORS, sizeof(VALKEY_GLIDE_CB_MIN_ERRORS) - 1);
config->circuit_breaker->min_errors = min_errors_zv
? (uint32_t) zval_get_long(min_errors_zv)
: VALKEY_GLIDE_CB_DEFAULT_MIN_ERRORS;

/* Parse open_timeout_ms (optional, default 5000) */
zval* open_timeout_zv = zend_hash_str_find(
cb_ht, VALKEY_GLIDE_CB_OPEN_TIMEOUT_MS, sizeof(VALKEY_GLIDE_CB_OPEN_TIMEOUT_MS) - 1);
config->circuit_breaker->open_timeout_ms = open_timeout_zv
? (uint32_t) zval_get_long(open_timeout_zv)
: VALKEY_GLIDE_CB_DEFAULT_OPEN_TIMEOUT_MS;

/* Parse count_timeouts (optional, default false) */
zval* count_timeouts_zv = zend_hash_str_find(
cb_ht, VALKEY_GLIDE_CB_COUNT_TIMEOUTS, sizeof(VALKEY_GLIDE_CB_COUNT_TIMEOUTS) - 1);
config->circuit_breaker->count_timeouts = count_timeouts_zv
? zval_is_true(count_timeouts_zv)
: VALKEY_GLIDE_CB_DEFAULT_COUNT_TIMEOUTS;

/* Parse consecutive_successes (optional, default 3) */
zval* consecutive_zv =
zend_hash_str_find(cb_ht,
VALKEY_GLIDE_CB_CONSECUTIVE_SUCCESSES,
sizeof(VALKEY_GLIDE_CB_CONSECUTIVE_SUCCESSES) - 1);
config->circuit_breaker->consecutive_successes =
consecutive_zv ? (uint32_t) zval_get_long(consecutive_zv)
: VALKEY_GLIDE_CB_DEFAULT_CONSECUTIVE_SUCCESSES;
} else {
config->circuit_breaker = NULL;
}

config->address_resolver = params->address_resolver;

_initialize_open_telemetry(params, is_cluster);
Expand Down Expand Up @@ -623,6 +683,14 @@ PHP_MINIT_FUNCTION(valkey_glide) {
return FAILURE;
}

/* CircuitBreakerException class */
valkey_glide_circuit_breaker_exception_ce =
register_class_CircuitBreakerException(valkey_glide_exception_ce);
if (!valkey_glide_circuit_breaker_exception_ce) {
php_error_docref(NULL, E_ERROR, "Failed to register CircuitBreakerException class");
return FAILURE;
}

/* Set object creation handlers */
if (valkey_glide_ce) {
valkey_glide_ce->create_object = create_valkey_glide_object;
Expand Down Expand Up @@ -780,6 +848,11 @@ void valkey_glide_cleanup_client_config(valkey_glide_base_client_configuration_t
config->client_side_cache = NULL;
}

if (config->circuit_breaker) {
efree(config->circuit_breaker);
config->circuit_breaker = NULL;
}

/* address_resolver is a borrowed reference from PHP, don't free it */
config->address_resolver = NULL;
}
Expand Down Expand Up @@ -890,7 +963,8 @@ static int valkey_glide_create_connection(valkey_glide_object* valkey_glide,
zval* context,
zval* compression,
zval* client_side_cache,
zval* address_resolver) {
zval* address_resolver,
zval* circuit_breaker) {
valkey_glide_php_common_constructor_params_t common_params;
valkey_glide_init_common_constructor_params(&common_params);

Expand Down Expand Up @@ -937,6 +1011,7 @@ static int valkey_glide_create_connection(valkey_glide_object* valkey_glide,
common_params.compression = compression;
common_params.client_side_cache = client_side_cache;
common_params.address_resolver = address_resolver;
common_params.circuit_breaker = circuit_breaker;

/* Default to localhost:6379 if no addresses provided */
zval addresses_array;
Expand Down Expand Up @@ -1056,8 +1131,9 @@ PHP_METHOD(ValkeyGlide, connect) {
zval* compression = NULL;
zval* client_side_cache = NULL;
zval* address_resolver = NULL;
zval* circuit_breaker = NULL;

ZEND_PARSE_PARAMETERS_START(0, 21)
ZEND_PARSE_PARAMETERS_START(0, 22)
Z_PARAM_OPTIONAL
Z_PARAM_STRING_OR_NULL(host, host_len)
Z_PARAM_ZVAL_OR_NULL(port_zval)
Expand All @@ -1080,6 +1156,7 @@ PHP_METHOD(ValkeyGlide, connect) {
Z_PARAM_ARRAY_OR_NULL(compression)
Z_PARAM_ARRAY_OR_NULL(client_side_cache)
Z_PARAM_ZVAL_OR_NULL(address_resolver)
Z_PARAM_ARRAY_OR_NULL(circuit_breaker)
ZEND_PARSE_PARAMETERS_END_EX(RETURN_THROWS());

/* Apply defaults for nullable parameters */
Expand Down Expand Up @@ -1153,7 +1230,8 @@ PHP_METHOD(ValkeyGlide, connect) {
context,
compression,
client_side_cache,
address_resolver);
address_resolver,
circuit_breaker);

/* Clean up temporary addresses array if we created it */
if (host != NULL) {
Expand Down
Loading
Loading