Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor bug fix for shell scripts in sandbox directory #898

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

lpstudy
Copy link

@lpstudy lpstudy commented Jun 1, 2017

(#897)

  1. deploy.sh: extra x in "$1"x == "master_slave". Also add -h option which is common in linux shell scripts.
  2. start_bfs.sh: bfs can not work well with raft mode enabled with only sleeping 5 seconds, add 'sleep 30' when using raft mode.
  3. start_bfs.sh: cd - print the working directory which is useless. Add the server starting message like start nameserver0 with pid xxx.
  4. clear.sh: redirect the error log message like process not found to /dev/null. It does no good but confuses the users.

lpstudy and others added 4 commits February 12, 2017 18:06
(baidu#897)
deploy.sh:    extra x in "$1"x == "master_slave". Also add `-h` option which is common in linux shell scripts.
start_bfs.sh: bfs can not work well with raft mode enabled with only sleeping 5 seconds, add 'sleep 30' when using raft mode.
clear.sh:     redirect the error log message like `process not found` to `/dev/null`. It does no good but confuses the users.
start_bfs.sh: `cd -` print the working directory which is useless. Add the server starting message like `start nameserver0 with pid xxx`
@CLAassistant
Copy link

CLAassistant commented Jun 1, 2017

CLA assistant check
All committers have signed the CLA.

@@ -10,8 +10,16 @@ if [ "$1"x = "x" ]; then
ns_num=1;
elif [ "$1x" == "raftx" ]; then
ns_num=3
elif [ "$1x" == "master_slave" ]; then
elif [ "$1" == "master_slave" ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

elif [ "$1x" == "master_slavex" ]; then?

Copy link
Author

Choose a reason for hiding this comment

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

I think each one is ok! when we use "", it is OK to use "$1"even when $1 == NULL.
Traditionally, we can use [ x$1 == x ] or [ "$1" == "" ], but combining them together seems werid.
Of course, it is my own idea.

Copy link
Author

Choose a reason for hiding this comment

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

If possible, please solve this pull request. I am not familiar with git. And I have made a mistake, which creates the pull request from my master branch. It is really annoying.

src/flags.cc Outdated
DEFINE_int64(chunkserver_disk_safe_space, 5120, "If space left on a disk is less then this value, the disk will be concidered full. In MB");
DEFINE_int32(chunkserver_disk_buf_size, 100, "Base number of buffers which are in the waiting list. Used to computer disk workload");
DEFINE_int64(chunkserver_disk_safe_space, 5120, "If space left on a disk is less then this value, the disk will be considered full. In MB");
DEFINE_int32(chunkserver_disk_data_store_limit_percent, 95, "If the percentage of data stored on a disk is larger than this value, the disk will be considered full. E.g. 95 means 95 percent");
Copy link
Collaborator

Choose a reason for hiding this comment

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

建议跟脚本改动分俩pr,这样后看起来比较清晰

Copy link
Author

Choose a reason for hiding this comment

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

说实话,我不会整两个pr。 我create new pull request,但是立刻合并到这一个上面了。

Copy link
Collaborator

Choose a reason for hiding this comment

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

在本地新建一个分支,然后push到github上,然后再新开pr就可以了

@@ -335,7 +336,9 @@ double ChunkServerManager::GetChunkServerLoad(ChunkServerInfo* cs) {
double data_score = cs->data_size() * 1.0 / cs->disk_quota();
int64_t space_left = cs->disk_quota() - cs->data_size();

if (data_score > 0.95 || space_left < (5L << 30) || pending_score > 1.0) {
if (data_score > FLAGS_chunkserver_disk_data_store_limit_percent * 1.0 / 100
|| space_left < (5L << 30)
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里应该用FLAG吧

Copy link
Author

Choose a reason for hiding this comment

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

5L << 30是另外一个人提交的,应该交给他处理吧?

Copy link
Collaborator

Choose a reason for hiding this comment

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

嗯,只是这样改那边肯定就冲突了,一个个来吧,先分pr就可以了

Copy link
Author

@lpstudy lpstudy Jun 16, 2017

Choose a reason for hiding this comment

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

已经建立了新的pr, 903。
旧的这个commit已经被删除了,内容扔到了新的pr中。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants