[PR] remove old rpcz directory even if program crash (brpc)
yanglimingcn opened a new pull request, #2610: URL: https://github.com/apache/brpc/pull/2610 ### What problem does this PR solve? rpcz目录在正常运行过程会周期性的删除旧的数据，但是如果在程序运行一段时间crash掉或者被kill掉，原来的rpcz目录不会释放掉，日积月累会占用很多空间。 这个PR会在rpcz启动的时候并且rpcz_keep_span_db为false的情况下，清理旧的数据。 Issue Number: Problem Summary: ### What is changed and the side effects? Changed: Side effects: - Performance effects(性能影响): - Breaking backward compatibility(向后兼容性): --- ### Check List: - Please make sure your changes are compilable(请确保你的更改可以通过编译). - When providing us with a new feature, it is best to add related tests(如果你向我们增加一个新的功能, 请添加相关测试). - Please follow [Contributor Covenant Code of Conduct](../../master/CODE_OF_CONDUCT.md).(请遵循贡献者准则). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org For additional commands, e-mail: dev-h...@brpc.apache.org
Re: [PR] remove old rpcz directory even if program crash (brpc)
wwbmmm commented on code in PR #2610:
URL: https://github.com/apache/brpc/pull/2610#discussion_r1575911958
##
src/brpc/span.cpp:
##
@@ -506,6 +509,12 @@ inline uint64_t ToLittleEndian(const uint32_t* buf) {
}
SpanDB* SpanDB::Open() {
+// Remove old rpcz directory even if crash occurs.
+if (!FLAGS_rpcz_keep_span_db) {
+std::string cmd = butil::string_printf("rm -rf %s",
FLAGS_rpcz_database_dir.c_str());
Review Comment:
这样操作会不会太危险了，万一 FLAGS_rpcz_database_dir 被设置成 / 之类的。
是不是可以遍历下目录，删除里面的id.db和time.db文件就可以
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org
Re: [PR] remove old rpcz directory even if program crash (brpc)
chenBright commented on PR #2610: URL: https://github.com/apache/brpc/pull/2610#issuecomment-2071846974 master分支已经修复了ci的问题，可以更新一下PR。 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org For additional commands, e-mail: dev-h...@brpc.apache.org
Re: [PR] remove old rpcz directory even if program crash (brpc)
yanglimingcn commented on code in PR #2610:
URL: https://github.com/apache/brpc/pull/2610#discussion_r1580562181
##
src/brpc/span.cpp:
##
@@ -506,6 +509,12 @@ inline uint64_t ToLittleEndian(const uint32_t* buf) {
}
SpanDB* SpanDB::Open() {
+// Remove old rpcz directory even if crash occurs.
+if (!FLAGS_rpcz_keep_span_db) {
+std::string cmd = butil::string_printf("rm -rf %s",
FLAGS_rpcz_database_dir.c_str());
Review Comment:
嗯，我觉得你说的有道理，我也犹豫过。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org
Re: [PR] remove old rpcz directory even if program crash (brpc)
yanglimingcn commented on code in PR #2610:
URL: https://github.com/apache/brpc/pull/2610#discussion_r1580653847
##
src/brpc/span.cpp:
##
@@ -506,6 +509,12 @@ inline uint64_t ToLittleEndian(const uint32_t* buf) {
}
SpanDB* SpanDB::Open() {
+// Remove old rpcz directory even if crash occurs.
+if (!FLAGS_rpcz_keep_span_db) {
+std::string cmd = butil::string_printf("rm -rf %s",
FLAGS_rpcz_database_dir.c_str());
Review Comment:
如果只删除id.db和time.db，目录下会遗留很多类似这样20240205.083927.20680的空目录。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org
Re: [PR] remove old rpcz directory even if program crash (brpc)
wwbmmm commented on code in PR #2610:
URL: https://github.com/apache/brpc/pull/2610#discussion_r1582486422
##
src/brpc/span.cpp:
##
@@ -506,6 +509,12 @@ inline uint64_t ToLittleEndian(const uint32_t* buf) {
}
SpanDB* SpanDB::Open() {
+// Remove old rpcz directory even if crash occurs.
+if (!FLAGS_rpcz_keep_span_db) {
+std::string cmd = butil::string_printf("rm -rf %s",
FLAGS_rpcz_database_dir.c_str());
Review Comment:
可以再检测下id.db的上级目录是否为空，如果为空则删除，不为空则保留
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org
Re: [PR] remove old rpcz directory even if program crash (brpc)
yanglimingcn commented on code in PR #2610:
URL: https://github.com/apache/brpc/pull/2610#discussion_r1582806405
##
src/brpc/span.cpp:
##
@@ -506,6 +509,12 @@ inline uint64_t ToLittleEndian(const uint32_t* buf) {
}
SpanDB* SpanDB::Open() {
+// Remove old rpcz directory even if crash occurs.
+if (!FLAGS_rpcz_keep_span_db) {
+std::string cmd = butil::string_printf("rm -rf %s",
FLAGS_rpcz_database_dir.c_str());
Review Comment:
就是判断上层目录是否是“/”，如果不是就可以删除是这个意思吗？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org
Re: [PR] remove old rpcz directory even if program crash (brpc)
yanglimingcn commented on code in PR #2610:
URL: https://github.com/apache/brpc/pull/2610#discussion_r1585752728
##
src/brpc/span.cpp:
##
@@ -506,6 +509,12 @@ inline uint64_t ToLittleEndian(const uint32_t* buf) {
}
SpanDB* SpanDB::Open() {
+// Remove old rpcz directory even if crash occurs.
+if (!FLAGS_rpcz_keep_span_db) {
+std::string cmd = butil::string_printf("rm -rf %s",
FLAGS_rpcz_database_dir.c_str());
Review Comment:
目前改成了增则表达式的方式。也可以改成，比如：FLAGS_rpcz_database_dir的配置为./rpc_data，在创建的时候bRPC在后边加上rpcz，实际目录就是
“./rpc_data/rpcz”。这样不管用户怎么配置，bRPC都加上了rpcz的目录，删除的时候就没有问题了。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org
Re: [PR] remove old rpcz directory even if program crash (brpc)
wwbmmm commented on code in PR #2610:
URL: https://github.com/apache/brpc/pull/2610#discussion_r1590484646
##
src/brpc/span.cpp:
##
@@ -506,6 +509,13 @@ inline uint64_t ToLittleEndian(const uint32_t* buf) {
}
SpanDB* SpanDB::Open() {
+// Remove old rpcz directory even if crash occurs.
+if (!FLAGS_rpcz_keep_span_db) {
+std::string cmd = butil::string_printf("rm -rf
%s/[0-9]*\.*[0-9]*\.[0-9]*",
Review Comment:
最好不要用拼接命令行的方式，可以用文件系统函数遍历一下FLAGS_rpcz_database_dir下的子目录，然后判断目录名符合正则表达式的目录，再进行删除
##
src/brpc/span.cpp:
##
@@ -506,6 +509,13 @@ inline uint64_t ToLittleEndian(const uint32_t* buf) {
}
SpanDB* SpanDB::Open() {
+// Remove old rpcz directory even if crash occurs.
+if (!FLAGS_rpcz_keep_span_db) {
+std::string cmd = butil::string_printf("rm -rf
%s/[0-9]*\.*[0-9]*\.[0-9]*",
Review Comment:
这样还是有命令注入的风险，比如FLAGS_rpcz_database_dir被改成 "/ abc"，中间有个空格，那么命令就变成了同时删除"/"
和"abc/[0-9]*\.*[0-9]*\.[0-9]*"这两种目录
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org
Re: [PR] remove old rpcz directory even if program crash (brpc)
yanglimingcn commented on code in PR #2610:
URL: https://github.com/apache/brpc/pull/2610#discussion_r1590493865
##
src/brpc/span.cpp:
##
@@ -506,6 +509,13 @@ inline uint64_t ToLittleEndian(const uint32_t* buf) {
}
SpanDB* SpanDB::Open() {
+// Remove old rpcz directory even if crash occurs.
+if (!FLAGS_rpcz_keep_span_db) {
+std::string cmd = butil::string_printf("rm -rf
%s/[0-9]*\.*[0-9]*\.[0-9]*",
Review Comment:
我用struct dirent *readdir(DIR *dirp);还是用Filesystem library (since C++17)合适呢？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org
Re: [PR] remove old rpcz directory even if program crash (brpc)
wwbmmm commented on code in PR #2610:
URL: https://github.com/apache/brpc/pull/2610#discussion_r1590507698
##
src/brpc/span.cpp:
##
@@ -506,6 +509,13 @@ inline uint64_t ToLittleEndian(const uint32_t* buf) {
}
SpanDB* SpanDB::Open() {
+// Remove old rpcz directory even if crash occurs.
+if (!FLAGS_rpcz_keep_span_db) {
+std::string cmd = butil::string_printf("rm -rf
%s/[0-9]*\.*[0-9]*\.[0-9]*",
Review Comment:
> 我用struct dirent *readdir(DIR *dirp);还是用Filesystem library (since C++17)合适呢？
有个封装好的 butil::FileEnumerator
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org
Re: [PR] remove old rpcz directory even if program crash (brpc)
yanglimingcn commented on PR #2610: URL: https://github.com/apache/brpc/pull/2610#issuecomment-2132556055 @wwbmmm 这个PR还有问题吗 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org For additional commands, e-mail: dev-h...@brpc.apache.org
Re: [PR] remove old rpcz directory even if program crash (brpc)
wwbmmm commented on PR #2610: URL: https://github.com/apache/brpc/pull/2610#issuecomment-2144584435 LGTM -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org For additional commands, e-mail: dev-h...@brpc.apache.org
Re: [PR] remove old rpcz directory even if program crash (brpc)
yanglimingcn merged PR #2610: URL: https://github.com/apache/brpc/pull/2610 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org For additional commands, e-mail: dev-h...@brpc.apache.org
