Skip to content

Conversation

iko1
Copy link
Contributor

@iko1 iko1 commented Jul 24, 2022

Fixes #209
Signed-off-by: iko1 [email protected]

@@ -199,6 +217,13 @@ Usage: dragonfly [FLAGS]
CHECK_LT(kver.major, 99u);
dfly::kernel_version = kver.kernel * 100 + kver.major;

string pidfile_path = GetFlag(FLAGS_pidfile);
Copy link
Collaborator

Choose a reason for hiding this comment

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

the feature is a bit more complicated than that. You also need to remove the file upon exit.
see https://github.com/redis/redis/blob/d00b8af89265c501fb4a0cdd546702b90432a896/src/server.c#L421

for example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you elaborate on your comment here? I'm unsure where to place the file deletion in dfly_main.cc and how to ensure the deletion function will be called at the server exit.

Copy link
Collaborator

Choose a reason for hiding this comment

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

what you did at lines 265-267 is good. Was it always there? I did not see it before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It just added in the latest commit after you asked for the removal of the file upon exit.

ofs.open(path, ios::trunc);
ofs << getpid();
ofs.close();
} catch (ios::failure& ex) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I also do not use c++ exceptions. These are poorly designed in c++ and you can see that I always return errors explicitly.

@iko1 iko1 force-pushed the feature/add-pidfile-parameter branch from d18cf06 to dd3deab Compare July 27, 2022 06:32
@romange romange changed the title feat(server): add pidfile parameter (#209) feat(server): add pidfile parameter. Jul 27, 2022
@romange romange merged commit 753c1c0 into dragonflydb:main Jul 27, 2022
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.

--pidfile parameter for the dragonflydb binary
2 participants