Skip to content

Commit

Permalink
Revert "*: deny unknown fields when deserializing config TOML (#5190)" (
Browse files Browse the repository at this point in the history
#5350)

Signed-off-by: kennytm <kennytm@gmail.com>
  • Loading branch information
kennytm authored and sre-bot committed Aug 28, 2019
1 parent adff3e1 commit 83a2357
Show file tree
Hide file tree
Showing 11 changed files with 7 additions and 61 deletions.
16 changes: 7 additions & 9 deletions CHANGELOG.md
Expand Up @@ -3,24 +3,22 @@ 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 @@ -40,15 +38,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 @@ -73,11 +71,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: 0 additions & 1 deletion components/pd_client/src/config.rs
Expand Up @@ -9,7 +9,6 @@ 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: 0 additions & 1 deletion components/tikv_util/src/security.rs
Expand Up @@ -11,7 +11,6 @@ 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: 0 additions & 9 deletions src/config.rs
Expand Up @@ -75,7 +75,6 @@ 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 @@ -125,7 +124,6 @@ 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 @@ -594,7 +592,6 @@ 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 @@ -635,7 +632,6 @@ 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 @@ -865,7 +861,6 @@ 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 @@ -982,7 +977,6 @@ 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 @@ -1030,7 +1024,6 @@ macro_rules! readpool_config {
($struct_name:ident, $test_mod_name:ident, $display_name:expr) => {
#[derive(Clone, Copy, 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 @@ -1238,7 +1231,6 @@ impl Default for CoprReadPoolConfig {

#[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 @@ -1255,7 +1247,6 @@ 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: 0 additions & 1 deletion src/import/config.rs
Expand Up @@ -5,7 +5,6 @@ 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: 0 additions & 1 deletion src/raftstore/coprocessor/config.rs
Expand Up @@ -5,7 +5,6 @@ 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: 0 additions & 1 deletion src/raftstore/store/config.rs
Expand Up @@ -9,7 +9,6 @@ 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: 0 additions & 1 deletion src/server/config.rs
Expand Up @@ -43,7 +43,6 @@ 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: 0 additions & 2 deletions src/storage/config.rs
Expand Up @@ -27,7 +27,6 @@ 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 @@ -67,7 +66,6 @@ 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: 0 additions & 1 deletion src/storage/lock_manager/config.rs
Expand Up @@ -4,7 +4,6 @@ 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: 0 additions & 34 deletions tests/integrations/config/mod.rs
Expand Up @@ -555,37 +555,3 @@ 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 83a2357

Please sign in to comment.