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.

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

Avoid folly::init overwirte log configuration. #4443

Merged
merged 4 commits into from
Jul 20, 2022
Merged

Avoid folly::init overwirte log configuration. #4443

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d3b93b9
Avoid folly::init overwirte log configuration.
Shylock-Hg Jul 19, 2022
7ce3ab0
Revert default config.
Shylock-Hg Jul 19, 2022
59475fe
Merge branch 'master' into fix/folly-init-overwirte-log-conf
Shylock-Hg Jul 19, 2022
5653444
Merge branch 'master' into fix/folly-init-overwirte-log-conf
Sophie-Xie Jul 19, 2022
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
14 changes: 8 additions & 6 deletions src/daemons/MetaDaemon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,7 @@ int main(int argc, char* argv[]) {
// the 2nd will make the 1st failed to output log anymore
Copy link
Contributor

Choose a reason for hiding this comment

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

Please verify if the behavior in the comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In current folly::init and SetUpLogging is called after pid checking too.

Copy link
Contributor Author

@Shylock-Hg Shylock-Hg Jul 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

But Standalone daemon and Graphd daemon don't require this constraint even before this PR, need to modify it?

Copy link
Contributor

Choose a reason for hiding this comment

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

It is ok for now. I can't clearly remember why we move setupLogging very close to start position of the main.

gflags::ParseCommandLineFlags(&argc, &argv, false);

// Setup logging
auto status = setupLogging(argv[0]);
if (!status.ok()) {
LOG(ERROR) << status;
return EXIT_FAILURE;
}
Status status;

#if defined(ENABLE_BREAKPAD)
status = setupBreakpad();
Expand Down Expand Up @@ -106,6 +101,13 @@ int main(int argc, char* argv[]) {
google::SetStderrLogging(google::INFO);
}

// Setup logging
status = setupLogging(argv[0]);
if (!status.ok()) {
LOG(ERROR) << status;
return EXIT_FAILURE;
}

if (FLAGS_daemonize) {
status = ProcessUtils::daemonize(pidPath);
if (!status.ok()) {
Expand Down
14 changes: 8 additions & 6 deletions src/daemons/StorageDaemon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,7 @@ int main(int argc, char *argv[]) {
// the 2nd will make the 1st failed to output log anymore
gflags::ParseCommandLineFlags(&argc, &argv, false);

// Setup logging
auto status = setupLogging(argv[0]);
if (!status.ok()) {
LOG(ERROR) << status;
return EXIT_FAILURE;
}
Status status;

#if defined(ENABLE_BREAKPAD)
status = setupBreakpad();
Expand Down Expand Up @@ -94,6 +89,13 @@ int main(int argc, char *argv[]) {
google::SetStderrLogging(google::INFO);
}

// Setup logging
status = setupLogging(argv[0]);
if (!status.ok()) {
LOG(ERROR) << status;
return EXIT_FAILURE;
}

if (FLAGS_daemonize) {
status = ProcessUtils::daemonize(pidPath);
if (!status.ok()) {
Expand Down