Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Added verbose output #52

Closed
wants to merge 3 commits into from

5 participants

@jamierumbelow

Hi there,

I got rid of the version message when -v is passed and now that enables verbose output, where incoming messages and other info will be passed into stdout. I saw this in the TODO doc and thought I'd chip in.

It's been a very long time since I've done any C so you'll have to forgive me... my skills are a little rusty.

Thanks,

Jamie

@jamierumbelow jamierumbelow Removed the version flag and replaced -v with verbosity. beanstalkd w…
…ill now output incoming messages and other info to stdout if -v is passed
3ca2d0e
@kr
Owner
kr commented

Thanks for the contribution! Before I can merge this in, there are just a couple of problems to address:

  • We can't get rid of (or change) the existing -v flag, but we can add a new one. I'd suggest -V (capital) for verbose.
  • This patch actually does two things: it adds a flag, and it adds log messages. Ideally is should be broken up into two (or more) commits. Let's just get the flag in, then we can worry about the actual log messages separately.
  • This code won't compile as it stands -- you've changed the type of prot_init but haven't updated the call or the declaration in prot.h include the new parameter. Also, prot_init's verbose variable is an int*; it should probably be an int.
  • More fundamentally, the verbose variable ought to be shared bwtween files. I'd suggest putting the storage of that variable in util.c, and declaring it as extern in util.h See the declarations of progname for an example.
  • There's some noise. This change introduces unnecessary whitespace changes and it has unrelated change to host_addr in beanstalkd.c. If indeed host_addr ought to be initialized, that should go in a separate commit.
@jamierumbelow

Hi,

Thanks for the feedback! Everything compiles are runs for me, which is very strange. But anyway, I've implemented your suggestions and moved verbose into a different flag (-V) and moved the declaration over to util.h.

As far as whitespace goes, that's just in there from debugging and testing and moving things about... I'm not particularly sure why people are bothered by this. I guess it's to keep commits clean, but anyway, I'm not really sure what I can do to rectify this. Can you pick specific changes from a changeset and merge into a new commit?

I'd really like to start contributing to more open source projects in different languages, so please bare with me, and I'll be properly on my feet soon.

Thanks again,

Jamie

@AlekSi

Any updates? Verbose mode would be very helpful.

@halfdan

+1 for this

@kr
Owner
kr commented

I'm doing some serious refactoring right now.
I will try to merge this in as soon as that is done.

@kr kr was assigned
@philsturgeon

+1 for this. I'd test it myself but the code seems to have vanished. Did you delete this repo Jamie?

@kr kr closed this pull request from a commit
@kr verbose mode; closes #52 d8cadfc
@kr kr closed this in d8cadfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 6, 2010
  1. @jamierumbelow

    Removed the version flag and replaced -v with verbosity. beanstalkd w…

    jamierumbelow authored
    …ill now output incoming messages and other info to stdout if -v is passed
Commits on Dec 10, 2010
  1. @jamierumbelow
  2. @jamierumbelow
This page is out of date. Refresh to see the latest.
Showing with 19 additions and 22 deletions.
  1. +16 −19 beanstalkd.c
  2. +2 −3 prot.c
  3. +1 −0  util.h
View
35 beanstalkd.c
@@ -1,20 +1,4 @@
-/* beanstalk - fast, general-purpose work queue */
-/* Copyright (C) 2007 Keith Rarick and Philotic Inc.
-
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
-
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
-
- * You should have received a copy of the GNU General Public License
- * along with this program. If not, see <http://www.gnu.org/licenses/>.
- */
#include "config.h"
@@ -44,6 +28,7 @@ static char *user = NULL;
static int detach = 0;
static char *port = "11300";
static char *host_addr;
+int verbose = 0;
static void
nullfd(int fd, int flags)
@@ -61,6 +46,7 @@ dfork()
pid_t p;
p = fork();
+
if (p == -1) exit(1);
if (p) exit(0);
}
@@ -102,6 +88,8 @@ su(const char *user) {
void
exit_cleanly(int sig)
{
+ if (verbose) printf("Shutting down. Bye!\n");
+
binlog_shutdown();
exit(0);
}
@@ -179,7 +167,8 @@ usage(char *msg, char *arg)
#ifndef HAVE_POSIX_FALLOCATE
" (will be rounded up to a multiple of 512 bytes)\n"
#endif
- " -v show version information\n"
+ " -V output verbosely"
+ " -v show version number\n"
" -h show this help\n",
progname, JOB_DATA_SIZE_LIMIT_DEFAULT, BINLOG_SIZE_LIMIT_DEFAULT);
exit(arg ? 5 : 0);
@@ -251,6 +240,9 @@ opts(int argc, char **argv)
case 'b':
binlog_dir = require_arg("-b", argv[++i]);
break;
+ case 'V':
+ verbose = 1;
+ break;
case 'h':
usage(NULL, NULL);
case 'v':
@@ -272,6 +264,8 @@ main(int argc, char **argv)
progname = argv[0];
opts(argc, argv);
+ if (verbose) printf("Loading beanstalkd...\n");
+
if (detach && binlog_dir) {
if (binlog_dir[0] != '/') {
warnx("The -b option requires an absolute path when used with -d.");
@@ -279,9 +273,9 @@ main(int argc, char **argv)
}
}
- job_init();
+ job_init();
prot_init();
-
+
/* We want to make sure that only one beanstalkd tries to use the binlog
* directory at a time. So acquire a lock now and never release it. */
if (binlog_dir) {
@@ -292,6 +286,8 @@ main(int argc, char **argv)
r = make_server_socket(host_addr, port);
if (r == -1) twarnx("make_server_socket()"), exit(111);
+ if (verbose) printf("Server listening on %s:%s\n", host_addr, port);
+
if (user) su(user);
ev_base = event_init();
set_sig_handlers();
@@ -304,6 +300,7 @@ main(int argc, char **argv)
prot_replay_binlog(&binlog_jobs);
if (detach) {
+ if (verbose) printf("Daemonizing... bye!\n");
daemonize();
event_reinit(ev_base);
}
View
5 prot.c
@@ -250,11 +250,9 @@ static int drain_mode = 0;
static usec started_at;
static uint64_t op_ct[TOTAL_OPS], timeout_ct = 0;
-
/* Doubly-linked list of connections with at least one reserved job. */
static struct conn running = { &running, &running, 0 };
-#ifdef DEBUG
static const char * op_names[] = {
"<unknown>",
CMD_PUT,
@@ -281,7 +279,6 @@ static const char * op_names[] = {
CMD_QUIT,
CMD_PAUSE_TUBE
};
-#endif
static job remove_buried_job(job j);
@@ -1179,6 +1176,7 @@ dispatch_cmd(conn c)
type = which_cmd(c);
dprintf("got %s command: \"%s\"\n", op_names[(int) type], c->cmd);
+ if (verbose) printf("got %s command: \"%s\"\n", op_names[(int) type], c->cmd);
switch (type) {
case OP_PUT:
@@ -1812,6 +1810,7 @@ h_accept(const int fd, const short which, struct event *ev)
if (!c) return twarnx("make_conn() failed"), close(cfd), brake();
dprintf("accepted conn, fd=%d\n", cfd);
+ if (verbose) printf("accepted conn, fd=%d\n", cfd);
r = conn_set_evq(c, EV_READ | EV_PERSIST, (evh) h_conn);
if (r == -1) return twarnx("conn_set_evq() failed"), close(cfd), brake();
}
View
1  util.h
@@ -36,6 +36,7 @@ void warn(const char *fmt, ...);
void warnx(const char *fmt, ...);
extern char *progname;
+extern int verbose;
#define twarn(fmt, args...) warn("%s:%d in %s: " fmt, \
__FILE__, __LINE__, __func__, ##args)
Something went wrong with that request. Please try again.