cst/inv: Create helpers to conditionally create inventory config #16636
Conversation
abhijat
force-pushed
the
feat/inv-scrub/check-config-exists
branch
4 times, most recently
from
1560e8d
to
81b58f6
Compare
|
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45149#018dc56f-cacc-452a-895d-5226b6605dff ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45236#018dd05f-606b-4053-938a-5edb92334e4d ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45236#018dd070-43a3-42a2-a0e3-79e6596333aa |
A helper is added to check if an inventor configuration exists already. This is expected to be used as a check before creating the inventory configuration. While the GET call used for the check returns the inventory config details, we do not use these other than the HTTP status code. For the AWS implementation added here the details are not needed. For GCP we will need to extract the server assigned UUID for future calls.
abhijat
force-pushed
the
feat/inv-scrub/check-config-exists
branch
from
81b58f6
to
2fd8d25
Compare
A top level method is added to conditionally create inventory config if it does not already exist. This involves two operations, one download (to check if the config exists) and one upload (to create). The results of the two are merged into a success/failure enum class. This top level method is expected to be used by callers to create the config.
abhijat
force-pushed
the
feat/inv-scrub/check-config-exists
branch
from
2fd8d25
to
695e569
Compare
abhijat
changed the title
|
CI failure #15679 |
| auto dl_res = co_await remote.download_object( | ||
| {.transfer_details | ||
| = {.bucket = _bucket, .key = _inventory_key, .parent_rtc = parent_rtc}, | ||
| .payload = buf}); | ||
| co_return dl_res == download_result::success; | ||
| } |
There was a problem hiding this comment.
i imagine HEAD does not work for inventory files
There was a problem hiding this comment.
This is an object in the REST API sense, there is no file backing the config, so a GET request is the only option (https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketInventoryConfiguration.html)
There was a problem hiding this comment.
To be clearer, this is not the inventory file itself but the inventory schedule, IE a server side entity
| // This buffer is thrown away after the GET call returns, we might introduce | ||
| // some sort of struct (or reuse the aws_report_configuration used for | ||
| // serialization), but for AWS we generally do not need the actual report | ||
| // config, the HTTP status for the GET call is sufficient to tell us if the | ||
| // config exists. |
| auto dl_res = co_await remote.download_object( | ||
| {.transfer_details | ||
| = {.bucket = _bucket, .key = _inventory_key, .parent_rtc = parent_rtc}, | ||
| .payload = buf}); |
There was a problem hiding this comment.
is it slightly odd to use download_object here? seems like some sort of more raw/low-level GET verb used directly would be clearer? (btw no need to chagne this , just trying to understand)
There was a problem hiding this comment.
It is a little off, but remote currently doesn't have a pure GET operation, download_object was also added recently. The right thing to do would probably be to add a GET operation and make download_object a wrapper for it.
Introduces helpers to check if inventory configuration exists, and to conditionally create one. Maps the result of the individual calls to
inventory_creation_resultstates:successfailurealready_existsThe creation of config is expected to be performed by any node in cluster at startup. Instead of simply attempting to create and relying on the server side validation to prevent duplicates, we first do a check before creating the config. One node creates the config and the others see that the config is present and move on.
Handling races while creation
It is possible that multiple nodes try to create the config after they observe that the config is missing, since the calls to check and create are separate. In this case all but one nodes will fail to create the config. The failing nodes should return the status as
already_existsrather thanfailed. To achieve this, on failure the nodes once again check if the config exists and they lost the race to create it.Backports Required
Release Notes