Skip to content
This repository has been archived by the owner on Jun 23, 2022. It is now read-only.

feat: add check for app envs #353

Merged
merged 32 commits into from
Feb 4, 2020
Merged

Conversation

levy5307
Copy link
Contributor

@levy5307 levy5307 commented Dec 9, 2019

In meta server, we have no check for app env replica.write_throttling and replica.write_throttling_by_size now, so there is no hint message when user sets an invalid value.

acelyc111
acelyc111 previously approved these changes Jan 24, 2020
include/dsn/env/replica_envs.h Outdated Show resolved Hide resolved
src/dist/replication/meta_server/server_state.h Outdated Show resolved Hide resolved
src/dist/replication/meta_server/app_env_validator.h Outdated Show resolved Hide resolved
src/dist/replication/meta_server/app_env_validator.cpp Outdated Show resolved Hide resolved
src/dist/replication/meta_server/app_env_validator.cpp Outdated Show resolved Hide resolved
src/dist/replication/meta_server/app_env_validator.cpp Outdated Show resolved Hide resolved
return true;
}

bool check_write_throttling(const std::string &env_value, std::string &hint_message)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

你这个跟 throttling_controller::parse_from_env 代码有比较大的重复,meta 检查了参数,replica 实际上就不需要检查了。可以后续的 PR 删。

另外你这个检查其实忽略了一种 case:100K*delay*100,200K*delay*100 这种你会判断是对的,

Copy link
Contributor Author

@levy5307 levy5307 Feb 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

你这个跟 throttling_controller::parse_from_env 代码有比较大的重复,meta 检查了参数,replica 实际上就不需要检查了。可以后续的 PR 删。

另外你这个检查其实忽略了一种 case:100K*delay*100,200K*delay*100 这种你会判断是对的,

也不能删,万一meta是老的,replica是新的,replica的判断给删了就没有了检查。你说的那个case的检查也实现了

src/dist/replication/meta_server/app_env_validator.cpp Outdated Show resolved Hide resolved
neverchanje
neverchanje previously approved these changes Feb 3, 2020
@levy5307 levy5307 merged commit d0dab50 into XiaoMi:master Feb 4, 2020
@levy5307 levy5307 changed the title feat: add check for app env replica.write_throttling and replica.write_throttling_by_size feat: add check for app envs Feb 4, 2020
neverchanje pushed a commit that referenced this pull request Mar 31, 2020
@levy5307 levy5307 deleted the meta-env-check branch May 26, 2020 06:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants