Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add config setting to force new channel #159

Merged
merged 2 commits into from
Jun 5, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
10 changes: 9 additions & 1 deletion src/Cache/Internal/DataGrpcManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,27 @@
use Momento\Auth\ICredentialProvider;
use Momento\Cache\Interceptors\AgentInterceptor;
use Momento\Cache\Interceptors\AuthorizationInterceptor;
use Momento\Config\IConfiguration;

class DataGrpcManager
{
public ScsClient $client;
private Channel $channel;

public function __construct(ICredentialProvider $authProvider)
public function __construct(ICredentialProvider $authProvider, IConfiguration $configuration)
{
$endpoint = $authProvider->getCacheEndpoint();
$channelArgs = ["credentials" => ChannelCredentials::createSsl()];
if ($authProvider->getTrustedCacheEndpointCertificateName()) {
$channelArgs["grpc.ssl_target_name_override"] = $authProvider->getTrustedCacheEndpointCertificateName();
}
$forceNewChannel = $configuration
->getTransportStrategy()
->getGrpcConfig()
->getForceNewChannel();
if ($forceNewChannel) {
$channelArgs["force_new"] = $forceNewChannel;
}
$this->channel = new Channel($endpoint, $channelArgs);
$interceptors = [
new AuthorizationInterceptor($authProvider->getAuthToken()),
Expand Down
2 changes: 1 addition & 1 deletion src/Cache/Internal/ScsDataClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ public function __construct(IConfiguration $configuration, ICredentialProvider $
validateOperationTimeout($operationTimeoutMs);
$this->deadline_milliseconds = $operationTimeoutMs ?? self::$DEFAULT_DEADLINE_MILLISECONDS;
$this->timeout = $this->deadline_milliseconds * self::$TIMEOUT_MULTIPLIER;
$this->grpcManager = new DataGrpcManager($authProvider);
$this->grpcManager = new DataGrpcManager($authProvider, $configuration);
$this->setLogger($configuration->getLoggerFactory()->getLogger(get_class($this)));
}

Expand Down
16 changes: 15 additions & 1 deletion src/Config/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,21 @@ public function withTransportStrategy(ITransportStrategy $transportStrategy): IC
* @param int $clientTimeoutSecs The amount of time in seconds to wait before cancelling the request.
* @return IConfiguration Configuration object with specified client timeout
*/
public function withClientTimeout(int $clientTimeoutSecs): IConfiguration {
public function withClientTimeout(int $clientTimeoutSecs): IConfiguration
{
return new Configuration($this->loggerFactory, $this->transportStrategy->withClientTimeout($clientTimeoutSecs));
}

/**
* Creates a new instance of the Configuration object, updated to use the specified "force_new" value
* when creating a gRPC channel
*
* @param bool $forceNewChannel If set to boolean "true" value, gRPC channels will be constructed with
* "force_new" set to true
* @return IConfiguration Configuration object with the specified force new channel setting
*/
public function withForceNewChannel(bool $forceNewChannel): IConfiguration
Copy link
Contributor

Choose a reason for hiding this comment

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

For now I think we probably just expose this on the IGrpcConfiguration interface since it is very specific to gRPC; if it ends up being a really commonly used option then we can wire it all the way up to the main config interface in the future.

See my PR for numGrpcChannels to get an idea what I was thinking. Happy to discuss further though.

{
return new Configuration($this->loggerFactory, $this->transportStrategy->withForceNewChannel($forceNewChannel));
}
}
10 changes: 10 additions & 0 deletions src/Config/IConfiguration.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,14 @@ public function withTransportStrategy(ITransportStrategy $transportStrategy): IC
* @return IConfiguration Configuration object with specified client timeout
*/
public function withClientTimeout(int $clientTimeoutSecs): IConfiguration;

/**
* Creates a new instance of the Configuration object, updated to use the specified "force_new" value
* when creating a gRPC channel
*
* @param bool $forceNewChannel If set to boolean "true" value, gRPC channels will be constructed with
* "force_new" set to true
* @return IConfiguration Configuration object with the specified force new channel setting
*/
public function withForceNewChannel(bool $forceNewChannel): IConfiguration;
}
4 changes: 4 additions & 0 deletions src/Config/Transport/IGrpcConfiguration.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,8 @@ interface IGrpcConfiguration
public function getDeadlineMilliseconds(): ?int;

public function withDeadlineMilliseconds(int $deadlineMilliseconds): IGrpcConfiguration;

public function getForceNewChannel(): ?bool;

public function withForceNewChannel(bool $forceNewChannel) : IGrpcConfiguration;
}
2 changes: 2 additions & 0 deletions src/Config/Transport/ITransportStrategy.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,6 @@ public function withGrpcConfig(IGrpcConfiguration $grpcConfig);
public function withMaxIdleMillis(int $maxIdleMillis);

public function withClientTimeout(int $clientTimeout);

public function withForceNewChannel(bool $forceNewChannel);
}
16 changes: 14 additions & 2 deletions src/Config/Transport/StaticGrpcConfiguration.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ class StaticGrpcConfiguration implements IGrpcConfiguration
{

private ?int $deadlineMilliseconds;
private ?bool $forceNewChannel;

public function __construct(?int $deadlineMilliseconds = null)
public function __construct(?int $deadlineMilliseconds = null, ?bool $forceNewChannel = false)
{
$this->deadlineMilliseconds = $deadlineMilliseconds;
$this->forceNewChannel = $forceNewChannel;
}

public function getDeadlineMilliseconds(): int|null
Expand All @@ -20,6 +22,16 @@ public function getDeadlineMilliseconds(): int|null

public function withDeadlineMilliseconds(int $deadlineMilliseconds): StaticGrpcConfiguration
{
return new StaticGrpcConfiguration($deadlineMilliseconds);
return new StaticGrpcConfiguration($deadlineMilliseconds, $this->forceNewChannel);
}

public function getForceNewChannel(): bool|null
{
return $this->forceNewChannel;
}

public function withForceNewChannel(bool $forceNewChannel): StaticGrpcConfiguration
{
return new StaticGrpcConfiguration($this->deadlineMilliseconds, $forceNewChannel);
}
}
10 changes: 10 additions & 0 deletions src/Config/Transport/StaticTransportStrategy.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,14 @@ public function withClientTimeout(int $clientTimeout): StaticTransportStrategy
$this->maxIdleMillis
);
}

public function withForceNewChannel(bool $forceNewChannel): StaticTransportStrategy
{
return new StaticTransportStrategy(
$this->maxConcurrentRequests,
$this->grpcConfig->withForceNewChannel($forceNewChannel),
$this->loggerFactory,
$this->maxIdleMillis
);
}
}