Skip to content

Minor bug fix for shell scripts like kill_tera.sh, launch_tera.sh and teracli #1280

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.

Sign up for GitHub

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

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

Minor bug fix for shell scripts like kill_tera.sh, launch_tera.sh and teracli #1280

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
81edfb1
Minor bug fix for shell scripts.
lpstudy Jun 2, 2017
2f6a8c0
Minor bug fix for not passing the Travis CI build.
lpstudy Jun 3, 2017
c2bd8a4
(#1279)
lpstudy Jun 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions build.conf.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ LIBUNWIND_VERSION=0.99
GPERFTOOLS_VERSION=2.5
INS_VERSION=0.15
NOSE_VERSION=1.3.7
READLINE_VERSION=7.0

if [ $MIRROR == "china" ]; then
BOOST_URL=http://mirrors.tuna.tsinghua.edu.cn/macports/distfiles/boost/boost_${BOOST_VERSION}.tar.bz2
Expand All @@ -32,6 +33,7 @@ if [ $MIRROR == "china" ]; then
GPERFTOOLS_URL=https://github.com/00k/gperftools/raw/master/gperftools-${GPERFTOOLS_VERSION}.tar.gz
INS_URL=https://github.com/baidu/ins/archive/${INS_VERSION}.tar.gz
NOSE_URL=http://mirrors.163.com/gentoo/distfiles/nose-${NOSE_VERSION}.tar.gz
READLINE_URL=http://git.savannah.gnu.org/cgit/readline.git/snapshot/readline-${READLINE_VERSION}.tar.gz
elif [ $MIRROR == "origin" ]; then
BOOST_URL=http://downloads.sourceforge.net/project/boost/boost/1.58.0/boost_${BOOST_VERSION}.tar.bz2
PROTOBUF_URL=https://github.com/google/protobuf/releases/download/v${PROTOBUF_VERSION}/protobuf-${PROTOBUF_VERSION}.tar.bz2
Expand All @@ -45,6 +47,7 @@ elif [ $MIRROR == "origin" ]; then
GPERFTOOLS_URL=https://github.com/gperftools/gperftools/releases/download/gperftools-${GPERFTOOLS_VERSION}/gperftools-${GPERFTOOLS_VERSION}.tar.gz
INS_URL=https://github.com/baidu/ins/archive/${INS_VERSION}.tar.gz
NOSE_URL=https://pypi.python.org/packages/58/a5/0dc93c3ec33f4e281849523a5a913fa1eea9a3068acfa754d44d88107a44/nose-${NOSE_VERSION}.tar.gz
READLINE_URL=http://git.savannah.gnu.org/cgit/readline.git/snapshot/readline-${READLINE_VERSION}.tar.gz
elif [ $MIRROR == "baidu" ]; then
BOOST_URL=http://gitlab.baidu.com/baidups/third/raw/master/boost_${BOOST_VERSION}.tar.bz2
PROTOBUF_URL=http://gitlab.baidu.com/baidups/third/raw/master/protobuf-${PROTOBUF_VERSION}.tar.bz2
Expand All @@ -58,6 +61,7 @@ elif [ $MIRROR == "baidu" ]; then
GPERFTOOLS_URL=http://gitlab.baidu.com/baidups/third/raw/master/gperftools-${GPERFTOOLS_VERSION}.tar.gz
INS_URL=http://gitlab.baidu.com/baidups/third/raw/master/ins-${INS_VERSION}.tar.gz
NOSE_URL=http://gitlab.baidu.com/baidups/third/raw/master/nose-${NOSE_VERSION}.tar.gz
READLINE_URL=http://git.savannah.gnu.org/cgit/readline.git/snapshot/readline-${READLINE_VERSION}.tar.gz
else
Copy link
Collaborator

Choose a reason for hiding this comment

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

百度内部编译方式不需要使用此包

Copy link
Author

Choose a reason for hiding this comment

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

已去掉

return 1
fi
Expand Down
17 changes: 16 additions & 1 deletion build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ if [ ${NOSE_VERSION} == "DISABLE" ]; then
echo "Disable nose."
elif [ ! -f "${FLAG_DIR}/nose_${NOSE_VERSION}" ] \
|| [ ! -f "${DEPS_PREFIX}/bin/nosetests" ] \
|| [ ! -d "${DEPS_PREFIX}/lib/nose" ]; then
|| [ ! -d "${DEPS_PREFIX}/lib/"nose* ]; then
wget --no-check-certificate -O nose-${NOSE_VERSION}.tar.gz ${NOSE_URL}
tar zxf nose-${NOSE_VERSION}.tar.gz --recursive-unlink
cd nose-${NOSE_VERSION}
Expand All @@ -233,6 +233,21 @@ elif [ ! -f "${FLAG_DIR}/nose_${NOSE_VERSION}" ] \
touch "${FLAG_DIR}/nose_${NOSE_VERSION}"
fi

# readline (teracli_main.cc use this and lead to compile failed)
if [ ${READLINE_VERSION} == "DISABLE" ]; then
echo "Disable readline."
elif [ ! -f "${FLAG_DIR}/readline_${READLINE_VERSION}" ] \
|| [ ! -f "${DEPS_PREFIX}/lib/libreadline.a" ] \
|| [ ! -d "${DEPS_PREFIX}/include/readline" ]; then
wget --no-check-certificate -O readline-${READLINE_VERSION}.tar.gz ${READLINE_URL}
tar zxf readline-${READLINE_VERSION}.tar.gz --recursive-unlink
cd readline-${READLINE_VERSION}
./configure ${DEPS_CONFIG} CPPFLAGS=-I${DEPS_PREFIX}/include LDFLAGS=-L${DEPS_PREFIX}/lib
make install
cd -
touch "${FLAG_DIR}/readline_${READLINE_VERSION}"
fi

Copy link
Collaborator

Choose a reason for hiding this comment

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

如果系统已经预装了readline，就不需要下载此tar包重新编译了

Copy link
Author

Choose a reason for hiding this comment

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

已经增加了判断，我的是centos7.2的系统，但是并没有安装readline库。需要手动安装，其他预装库都正常，我认为其他人也会有相似的情况。

cd ${WORK_DIR}

########################################
Expand Down
3 changes: 2 additions & 1 deletion example/onebox/bin/kill_tera.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#!/bin/bash
source ./config
CURRENT_DIR=`dirname $0`
source ${CURRENT_DIR}/config

PID=`ps x | grep tera_master | grep $PORT | awk '{print $1}'`;
if [ ${PID}"x" != "x" ]; then
Expand Down
2 changes: 1 addition & 1 deletion example/onebox/bin/launch_tera.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ CURRENT_DIR=`dirname $0`
source ${CURRENT_DIR}/config

# make sure tera is killed
./kill_tera.sh
./$CURRENT_DIR/kill_tera.sh

FAKE_ZK_PATH_PREFIX="${CURRENT_DIR}/../fakezk"
TIME=`date +%Y-%m-%d-%H:%M:%S`
Expand Down
18 changes: 18 additions & 0 deletions src/common/file/file_path.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>
#include <libgen.h>

Copy link
Collaborator

Choose a reason for hiding this comment

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

google的代码风格

Copy link
Author

Choose a reason for hiding this comment

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

感谢评论，原来我并没有关注过头文件的包含顺序问题，去调查了一下顺序：OS SDK .h , C标准库、C++标准库、其它库的头文件、自己工程的头文件。于是按照这个顺序重新组织了一下，如下：

#include "common/file/file_path.h"

#include <dirent.h>
#include <grp.h>
#include <libgen.h>
#include <pwd.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>

#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#include <gflags/gflags.h>

#include "common/base/string_ext.h"

#include <gflags/gflags.h>

Expand Down Expand Up @@ -105,6 +106,23 @@ bool CreateDirWithRetry(const std::string& dir_path) {
}
return is_success;
}
std::string GetCWD(){
char buf[1024];
if(getcwd(buf, 1024) == NULL){
return "";
}
return buf;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

代码风格：缩进不对，左大括号缺少空格

Copy link
Author

Choose a reason for hiding this comment

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

已修改

std::string GetProcessDir(){
char buf[1024];
ssize_t count = readlink("/proc/self/exe", buf, 1024);
Copy link
Collaborator

Choose a reason for hiding this comment

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

路径长度最大是1024吗

Copy link
Author

Choose a reason for hiding this comment

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

已设置为linux/limits.h中的PATH_MAX常量

if(count == 0) {
return "";
} else{
return dirname(buf);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

dirname返回一个char*，是否需要释放；如果不用释放，是否是线程安全的？

Copy link
Author

Choose a reason for hiding this comment

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

不需要释放，线程安全的。dirname可能返回内部的static alloc的内存，也可能返回buf的内存。buf本身是局部变量，可以确保线程安全，dirname本身是线程安全函数。

Copy link
Collaborator

Choose a reason for hiding this comment

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

如果是内部的静态分配，为什么是线程安全的

}

Copy link
Collaborator

Choose a reason for hiding this comment

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

同上

Copy link
Author

Choose a reason for hiding this comment

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

如上

std::string UidToName(uid_t uid) {
struct passwd *temp = NULL;
Expand Down
3 changes: 3 additions & 0 deletions src/common/file/file_path.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ std::string GetPathPrefix(const std::string& full_path,

bool CreateDirWithRetry(const std::string& dir_path);

std::string GetCWD();
std::string GetProcessDir();

std::string GidToName(gid_t gid);

std::string UidToName(uid_t uid);
Expand Down
8 changes: 6 additions & 2 deletions src/sdk/client_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1195,7 +1195,7 @@ static int InitFlags(const std::string& confpath, const std::string& log_prefix)
LOG(ERROR) << "should specify no more than one config file";
return -1;
}

std::string exedir = GetProcessDir();
if (!confpath.empty() && IsExist(confpath)){
Copy link
Collaborator

Choose a reason for hiding this comment

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

tera.flag里面会配置logdir，路径不对的话，会导致日志不能输出，可以增加一下判断

Copy link
Author

Choose a reason for hiding this comment

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

增加了判断。

flagfile = confpath;
} else if(!confpath.empty() && !IsExist(confpath)){
Expand All @@ -1211,12 +1211,16 @@ static int InitFlags(const std::string& confpath, const std::string& log_prefix)
flagfile = "./tera.flag";
} else if (IsExist("../conf/tera.flag")) {
flagfile = "../conf/tera.flag";
} else if (IsExist(exedir + "/./tera.flag")) {
flagfile = exedir + "/./tera.flag";
} else if (IsExist(exedir + "/../conf/tera.flag")) {
flagfile = exedir + "/../conf/tera.flag";
} else if (IsExist(utils::GetValueFromEnv("TERA_CONF"))) {
flagfile = utils::GetValueFromEnv("TERA_CONF");
} else {
LOG(ERROR) << "hasn't specify the flagfile, but default config file not found";
return -1;
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

代码风格

Copy link
Author

Choose a reason for hiding this comment

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

已修改

utils::LoadFlagFile(flagfile);

Expand Down