added --bs-file option to filestore-server and vhost#6575
Conversation
|
Note This is an automated comment that will be appended during run. Note All workloads for linux-x86_64-relwithdebinfo have completed. Tip Planned checks for linux-x86_64-relwithdebinfo. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 736f1e97de
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| Opts.AddLongOption("bs-file") | ||
| .OptionalArgument("FILE") | ||
| .StoreResult(&BlobStorageConfig); |
There was a problem hiding this comment.
This registers --bs-file on every TOptionsYdbBase consumer, not just filestore: cloud/blockstore/libs/disk_agent/options.h also inherits TOptionsYdbBase, but its InitKikimrConfig still ignores Options->BlobStorageConfig and always initializes blob storage with only AddAvailabilityDomains(1). In disk-agent deployments, passing the newly accepted --bs-file will therefore be silently ignored instead of loading the requested blob storage service set; either apply it in the disk-agent initializer too or keep the option scoped to the binaries that actually consume it.
Useful? React with 👍 / 👎.
|
Note This is an automated comment that will be appended during run. Note All workloads for linux-x86_64-relwithdebinfo have completed. Tip Planned checks for linux-x86_64-relwithdebinfo. 🟢 linux-x86_64-relwithdebinfo target: cloud/tasks/,cloud/storage/ (test time: 255s): all tests PASSED for commit 27214ae.
🟢 linux-x86_64-relwithdebinfo target: cloud/disk_manager/ (test time: 279s): all tests PASSED for commit 27214ae.
🟢 linux-x86_64-relwithdebinfo target: cloud/blockstore/ (test time: 1380s): all tests PASSED for commit 27214ae.
🟢 linux-x86_64-relwithdebinfo target: cloud/filestore/ (test time: 5212s): all tests PASSED for commit 27214ae.
|
| bsConfig); | ||
| } else { | ||
| bsConfig.MutableServiceSet()->AddAvailabilityDomains(1); | ||
| } |
There was a problem hiding this comment.
Let's add support to https://github.com/ydb-platform/nbs/tree/main/cloud/storage/core/tools/common/go/configurator as well
There was a problem hiding this comment.
We decided to skip it because we don't need to set bs.txt for any cluster, and the TBlobStorageConfig protobuf is not as small as we initially thought
2 participants
Notes
This PR allows to specify blob storage configuration on command line, alike to other parameters.
It was required in context of BSProxy failure injection, which is enabled in blob_storage_config.
Issue
#6467