Skip to content

Commit

Permalink
alternator: configurable override for DescribeEndpoints
Browse files Browse the repository at this point in the history
The AWS C++ SDK has a bug (aws/aws-sdk-cpp#2554)
where even if a user specifies a specific enpoint URL, the SDK uses
DescribeEndpoints to try to "refresh" the endpoint. The problem is that
DescribeEndpoints can't return a scheme (http or https) and the SDK
arbitrarily picks https - making it unable to communicate with Alternator
over http. As an example, the new "dynamodb shell" (written in C++)
cannot communicate with Alternator running over http.

This patch adds a configuration option, "alternator_describe_endpoints",
which can be used to override what DescribeEndpoints does:

1. Empty string (the default) leaves the current behavior -
   DescribeEndpoints echos the request's "Host" header.

2. The string "disabled" disables the DescribeEndpoints (it will return
   an UnknownOperationException). This is how DynamoDB Local behaves,
   and the AWS C++ SDK and the Dynamodb Shell work well in this mode.

3. Any other string is a fixed string to be returned by DescribeEndpoints.
   It can be useful in setups that should return a known address.

Note that this patch does not, by default, change the current behaivor
of DescribeEndpoints. But it us the future to override its behavior
in a user experiences problems in the field - without code changes.

Fixes scylladb#14410.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
  • Loading branch information
nyh committed Jun 28, 2023
1 parent 8d1dfbf commit df024f2
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 0 deletions.
11 changes: 11 additions & 0 deletions alternator/executor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4360,6 +4360,17 @@ future<executor::request_return_type> executor::list_tables(client_state& client

future<executor::request_return_type> executor::describe_endpoints(client_state& client_state, service_permit permit, rjson::value request, std::string host_header) {
_stats.api_operations.describe_endpoints++;
// The alternator_describe_endpoints configuration can be used to disable
// the DescribeEndpoints operation, or set it to return a fixed string
std::string override = _proxy.data_dictionary().get_config().alternator_describe_endpoints();
if (!override.empty()) {
if (override == "disabled") {
_stats.unsupported_operations++;
return make_ready_future<request_return_type>(api_error::unknown_operation(
"DescribeEndpoints disabled by configuration (alternator_describe_endpoints=disabled)"));
}
host_header = std::move(override);
}
rjson::value response = rjson::empty_object();
// Without having any configuration parameter to say otherwise, we tell
// the user to return to the same endpoint they used to reach us. The only
Expand Down
7 changes: 7 additions & 0 deletions db/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -954,6 +954,13 @@ db::config::config(std::shared_ptr<db::extensions> exts)
, alternator_ttl_period_in_seconds(this, "alternator_ttl_period_in_seconds", value_status::Used,
60*60*24,
"The default period for Alternator's expiration scan. Alternator attempts to scan every table within that period.")
, alternator_describe_endpoints(this, "alternator_describe_endpoints", liveness::LiveUpdate, value_status::Used,
"",
"Overrides the behavior of Alternator's DescribeEndpoints operation. "
"An empty value (the default) means DescribeEndpoints will return "
"the same endpoint used in the request. The string 'disabled' "
"disables the DescribeEndpoints operation. Any other string is the "
"fixed value that will be returned by DescribeEndpoints operations.")
, abort_on_ebadf(this, "abort_on_ebadf", value_status::Used, true, "Abort the server on incorrect file descriptor access. Throws exception when disabled.")
, redis_port(this, "redis_port", value_status::Used, 0, "Port on which the REDIS transport listens for clients.")
, redis_ssl_port(this, "redis_ssl_port", value_status::Used, 0, "Port on which the REDIS TLS native transport listens for clients.")
Expand Down
1 change: 1 addition & 0 deletions db/config.hh
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ public:
named_value<uint32_t> alternator_streams_time_window_s;
named_value<uint32_t> alternator_timeout_in_ms;
named_value<double> alternator_ttl_period_in_seconds;
named_value<sstring> alternator_describe_endpoints;

named_value<bool> abort_on_ebadf;

Expand Down

0 comments on commit df024f2

Please sign in to comment.