feat: Introduce dataserver and coordinator configuration (part.1)#848
feat: Introduce dataserver and coordinator configuration (part.1)#848mattisonchao merged 15 commits intomainfrom
Conversation
6e8a1b1 to
2b16105
Compare
|
Hi @copilot |
|
@mattisonchao I've opened a new pull request, #854, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Pull request overview
This PR refactors the Oxia configuration system by introducing structured Options types for both DataServer and Coordinator components. The changes replace the previous conf.Config structs with a new options-based architecture that supports YAML configuration files and provides better structure for configuration management.
Changes:
- Introduced
oxiad/dataserver/option/option.goandoxiad/coordinator/option/option.gowith comprehensive options structs - Replaced
conf.Configusage across dataserver and coordinator with newOptionstypes - Updated TLS configuration to use
TLSOptionswith methods likeTryIntoClientTLSConf()andTryIntoServerTLSConf() - Enhanced authentication options to use
Providerfield instead ofProviderName - Added common observability options for metrics and logging
- Improved slice capacity pre-allocation in several places for performance
Reviewed changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| oxiad/dataserver/option/option.go | New dataserver options structure with server, storage, replication, and observability configuration |
| oxiad/coordinator/option/option.go | New coordinator options structure with cluster, server, controller, and metadata configuration |
| common/security/tls.go | Renamed TLSOption to TLSOptions, added YAML/JSON tags, and new conversion methods |
| oxiad/dataserver/server.go | Updated to use new Options struct instead of conf.Config |
| oxiad/coordinator/server.go | Refactored to use new Options with viper-based config loading |
| cmd/server/cmd.go | Updated CLI flags to work with new dataserver Options |
| cmd/coordinator/cmd.go | Updated CLI flags to work with new coordinator Options |
| oxiad/common/rpc/auth/authentication.go | Updated Options to use Provider field and added validation methods |
| common/constant/flag.go | New file with boolean flag constants |
| tests/* | Updated all test files to use new Options instead of conf.Config |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 42 out of 42 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 42 out of 42 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
02c9543 to
4deecf6
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
common/constant/flag.go:20
- The diff shows that this file was completely replaced with different content. The original file was a test file in the
coordinatorpackage withTestCoordinator_MarshalingError, but it was replaced with constants in theconstantpackage. This appears to be a mistake - the test file should not have been deleted and replaced with constant definitions. The constants should be in a new file, and the original test should remain or be moved to an appropriate location.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
### Motivation This pull request is part 2 for #848, supporting the YAML file configurations for both dataserver and coordinator, which will help configure the cluster with various configurations. such as TLS, auth, etc ### Modification - Introduce the dataserver and coordinator YAML file. - Introduce a configuration schema to help users configure it with IDEA. - Introduce a tool to help generate the schema based on the latest struct. `oxia config create-schema` - Add some tests for coverage. ### What next? - Support watching configurations to dynamically change some memory status.
Motivation
This PR focuses on establishing the foundational configuration for the DataServer and Coordinator components of Oxia. It introduces dedicated YAML configuration files to manage operational settings, thereby avoiding the need for numerous commands in the CLI. Plus, we could also rely on the watch mechanism for dynamic updating.
Modification
Optionsstruct for the dataserver instance.Optionsstruct for the coordinator instance.What's Next
dataserver.yamlconfiguration.coordinator.yamlconfiguration.cluster.yamlconfiguration to show how to configure the oxia cluster.Options.