Skip to content

Commit

Permalink
*: deny unknown fields when deserializing config TOML (tikv#5190)
Browse files Browse the repository at this point in the history
Signed-off-by: kennytm <kennytm@gmail.com>
  • Loading branch information
kennytm authored and sre-bot committed Aug 9, 2019
1 parent 1bbc0c3 commit e118ab6
Show file tree
Hide file tree
Showing 11 changed files with 61 additions and 7 deletions.
16 changes: 9 additions & 7 deletions CHANGELOG.md
Expand Up @@ -3,22 +3,24 @@ All notable changes to this project are documented in this file.
See also [TiDB Changelog](https://github.com/pingcap/tidb/blob/master/CHANGELOG.md) and [PD Changelog](https://github.com/pingcap/pd/blob/master/CHANGELOG.md).

## [Unreleased]

- Make several `INFO` logs `DEBUG` and refined some log messages. (https://github.com/tikv/tikv/pull/4768)
- Unrecognized configuration will now prevent TiKV from starting. (https://github.com/tikv/tikv/pull/5190)

## [3.0.0]

+ Engine
- Introduce Titan, a key-value plugin that improves write performance for
scenarios with value sizes greater than 1KiB, and relieves write
amplification in certain degrees
- Optimize memory management to reduce memory allocation and copying for `Iterator Key Bound Option`
- Optimize memory management to reduce memory allocation and copying for `Iterator Key Bound Option`
- Support `block cache` sharing among different column families

+ Server
- Support reversed `raw_scan` and `raw_batch_scan`
- Support batch receiving and sending Raft messages, improving TPS by 7% for write intensive scenarios
- Support getting monitoring information via HTTP
- Support Local Reader in RawKV to improve performance
- Support Local Reader in RawKV to improve performance
- Reduce context switch overhead from `batch commands`

+ Raftstore
Expand All @@ -38,15 +40,15 @@ See also [TiDB Changelog](https://github.com/pingcap/tidb/blob/master/CHANGELOG.

+ Coprocessor
- Refactor the computation framework to implement vector operators, computation
using vector expressions, and vector aggregations to improve performance
using vector expressions, and vector aggregations to improve performance
- Support providing operator execution status for the `EXPLAIN ANALYZE` statement
in TiDB
in TiDB
- Switch to the `work-stealing` thread pool model to reduce context switch cost

+ Misc
- Develop a unified log format specification with restructured log system to
facilitate collection and analysis by tools
- Add performance metrics related to configuration information and key bound crossing.
- Add performance metrics related to configuration information and key bound crossing.

## [3.0.0-rc.3]

Expand All @@ -71,11 +73,11 @@ See also [TiDB Changelog](https://github.com/pingcap/tidb/blob/master/CHANGELOG.
+ Coprocessor
- Improve coprocessor batch executor. [4877](https://github.com/tikv/tikv/pull/4877)

+ Transaction
+ Transaction
- Support `ResolveLockLite` to allow only resolving specified lock keys. [4882](https://github.com/tikv/tikv/pull/4882)
- Improve pessimistic lock transaction. [4889](https://github.com/tikv/tikv/pull/4889)

+ Tikv-ctl
+ Tikv-ctl
- Improve `bad-regions` and `tombstone` subcommands. [4862](https://github.com/tikv/tikv/pull/4862)

+ Misc
Expand Down
1 change: 1 addition & 0 deletions components/pd_client/src/config.rs
Expand Up @@ -9,6 +9,7 @@ use tikv_util::config::ReadableDuration;
/// for infinity, logging only every 10th duplicate error.
#[derive(Clone, Serialize, Deserialize, PartialEq, Debug)]
#[serde(default)]
#[serde(deny_unknown_fields)]
#[serde(rename_all = "kebab-case")]
pub struct Config {
/// The PD endpoints for the client.
Expand Down
1 change: 1 addition & 0 deletions components/tikv_util/src/security.rs
Expand Up @@ -11,6 +11,7 @@ use grpcio::{

#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
#[serde(default)]
#[serde(deny_unknown_fields)]
#[serde(rename_all = "kebab-case")]
pub struct SecurityConfig {
pub ca_path: String,
Expand Down
9 changes: 9 additions & 0 deletions src/config.rs
Expand Up @@ -75,6 +75,7 @@ fn memory_mb_for_cf(is_raft_db: bool, cf: &str) -> usize {

#[derive(Clone, Serialize, Deserialize, PartialEq, Debug)]
#[serde(default)]
#[serde(deny_unknown_fields)]
#[serde(rename_all = "kebab-case")]
pub struct TitanCfConfig {
pub min_blob_size: ReadableSize,
Expand Down Expand Up @@ -124,6 +125,7 @@ macro_rules! cf_config {
($name:ident) => {
#[derive(Clone, Serialize, Deserialize, PartialEq, Debug)]
#[serde(default)]
#[serde(deny_unknown_fields)]
#[serde(rename_all = "kebab-case")]
pub struct $name {
pub block_size: ReadableSize,
Expand Down Expand Up @@ -592,6 +594,7 @@ impl RaftCfConfig {

#[derive(Clone, Serialize, Deserialize, PartialEq, Debug)]
#[serde(default)]
#[serde(deny_unknown_fields)]
#[serde(rename_all = "kebab-case")]
// Note that Titan is still an experimental feature. Once enabled, it can't fall back.
// Forced fallback may result in data loss.
Expand Down Expand Up @@ -632,6 +635,7 @@ impl TitanDBConfig {

#[derive(Clone, Serialize, Deserialize, PartialEq, Debug)]
#[serde(default)]
#[serde(deny_unknown_fields)]
#[serde(rename_all = "kebab-case")]
pub struct DbConfig {
#[serde(with = "rocks_config::recovery_mode_serde")]
Expand Down Expand Up @@ -861,6 +865,7 @@ impl RaftDefaultCfConfig {
// But each instance will limit their background jobs according to their own max_background_jobs
#[derive(Clone, Serialize, Deserialize, PartialEq, Debug)]
#[serde(default)]
#[serde(deny_unknown_fields)]
#[serde(rename_all = "kebab-case")]
pub struct RaftDbConfig {
#[serde(with = "rocks_config::recovery_mode_serde")]
Expand Down Expand Up @@ -977,6 +982,7 @@ impl RaftDbConfig {

#[derive(Clone, Serialize, Deserialize, PartialEq, Debug)]
#[serde(default)]
#[serde(deny_unknown_fields)]
#[serde(rename_all = "kebab-case")]
pub struct MetricConfig {
pub interval: ReadableDuration,
Expand Down Expand Up @@ -1024,6 +1030,7 @@ macro_rules! readpool_config {
($struct_name:ident, $test_mod_name:ident, $display_name:expr) => {
#[derive(Clone, Serialize, Deserialize, PartialEq, Debug)]
#[serde(default)]
#[serde(deny_unknown_fields)]
#[serde(rename_all = "kebab-case")]
pub struct $struct_name {
pub high_concurrency: usize,
Expand Down Expand Up @@ -1202,6 +1209,7 @@ impl Default for CoprocessorReadPoolConfig {

#[derive(Clone, Serialize, Deserialize, PartialEq, Debug, Default)]
#[serde(default)]
#[serde(deny_unknown_fields)]
#[serde(rename_all = "kebab-case")]
pub struct ReadPoolConfig {
pub storage: StorageReadPoolConfig,
Expand All @@ -1218,6 +1226,7 @@ impl ReadPoolConfig {

#[derive(Clone, Serialize, Deserialize, PartialEq, Debug)]
#[serde(default)]
#[serde(deny_unknown_fields)]
#[serde(rename_all = "kebab-case")]
pub struct TiKvConfig {
#[serde(with = "log_level_serde")]
Expand Down
1 change: 1 addition & 0 deletions src/import/config.rs
Expand Up @@ -5,6 +5,7 @@ use std::result::Result;

#[derive(Clone, Serialize, Deserialize, PartialEq, Debug)]
#[serde(default)]
#[serde(deny_unknown_fields)]
#[serde(rename_all = "kebab-case")]
pub struct Config {
pub num_threads: usize,
Expand Down
1 change: 1 addition & 0 deletions src/raftstore/coprocessor/config.rs
Expand Up @@ -5,6 +5,7 @@ use tikv_util::config::ReadableSize;

#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
#[serde(default)]
#[serde(deny_unknown_fields)]
#[serde(rename_all = "kebab-case")]
pub struct Config {
/// When it is true, it will try to split a region with table prefix if
Expand Down
1 change: 1 addition & 0 deletions src/raftstore/store/config.rs
Expand Up @@ -9,6 +9,7 @@ use tikv_util::config::{ReadableDuration, ReadableSize};

#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
#[serde(default)]
#[serde(deny_unknown_fields)]
#[serde(rename_all = "kebab-case")]
pub struct Config {
// true for high reliability, prevent data loss when power failure.
Expand Down
1 change: 1 addition & 0 deletions src/server/config.rs
Expand Up @@ -43,6 +43,7 @@ pub enum GrpcCompressionType {
/// Configuration for the `server` module.
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
#[serde(default)]
#[serde(deny_unknown_fields)]
#[serde(rename_all = "kebab-case")]
pub struct Config {
#[serde(skip)]
Expand Down
2 changes: 2 additions & 0 deletions src/storage/config.rs
Expand Up @@ -27,6 +27,7 @@ const DEFAULT_SCHED_PENDING_WRITE_MB: u64 = 100;

#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
#[serde(default)]
#[serde(deny_unknown_fields)]
#[serde(rename_all = "kebab-case")]
pub struct Config {
pub data_dir: String,
Expand Down Expand Up @@ -66,6 +67,7 @@ impl Config {

#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
#[serde(default)]
#[serde(deny_unknown_fields)]
#[serde(rename_all = "kebab-case")]
pub struct BlockCacheConfig {
pub shared: bool,
Expand Down
1 change: 1 addition & 0 deletions src/storage/lock_manager/config.rs
Expand Up @@ -4,6 +4,7 @@ use std::error::Error;

#[derive(Clone, Serialize, Deserialize, PartialEq, Debug)]
#[serde(default)]
#[serde(deny_unknown_fields)]
#[serde(rename_all = "kebab-case")]
pub struct Config {
pub enabled: bool,
Expand Down
34 changes: 34 additions & 0 deletions tests/integrations/config/mod.rs
Expand Up @@ -554,3 +554,37 @@ fn test_block_cache_backward_compatible() {
+ cfg.raftdb.defaultcf.block_cache_size.0
);
}

#[test]
fn test_error_on_unrecognized_config() {
let contents = [
"unknown-field = 123\n",
"[unknown-dict]\ncontent = 123\n",
"[[unknown-array]]\ncontent = 123\n",
"[readpool]\nunknown-field = 123\n",
"[readpool.coprocessor]\nunknown-field = 123\n",
"[readpool.storage]\nunknown-field = 123\n",
"[server]\nunknown-field = 123\n",
"[storage]\nunknown-field = 123\n",
"[storage.block-cache]\nunknown-field = 123\n",
"[pd]\nunknown-field = 123\n",
"[metric]\nunknown-field = 123\n",
"[raftstore]\nunknown-field = 123\n",
"[coprocessor]\nunknown-field = 123\n",
"[rocksdb]\nunknown-field = 123\n",
"[rocksdb.titan]\nunknown-field = 123\n",
"[rocksdb.defaultcf]\nunknown-field = 123\n",
"[rocksdb.defaultcf.titan]\nunknown-field = 123\n",
"[rocksdb.writecf]\nunknown-field = 123\n",
"[rocksdb.lockcf]\nunknown-field = 123\n",
"[raftdb]\nunknown-field = 123\n",
"[raftdb.defaultcf]\nunknown-field = 123\n",
"[security]\nunknown-field = 123\n",
"[import]\nunknown-field = 123\n",
];

for content in &contents {
let result = toml::from_str::<TiKvConfig>(content);
assert!(result.is_err());
}
}

0 comments on commit e118ab6

Please sign in to comment.