Re: [Patch 1/3] CLD: End-to-end verbosity
On 03/31/2010 08:43 PM, Pete Zaitcev wrote: diff --git a/server/server.c b/server/server.c index 3208e0f..2d68ee6 100644 --- a/server/server.c +++ b/server/server.c @@ -55,7 +55,7 @@ static struct argp_option options[] = { Store database environment in DIRECTORY. Default: CLD_DEF_DATADIR }, { debug, 'D', LEVEL, 0, - Set debug output to LEVEL (0 = off, 2 = max) }, + Set debug output to LEVEL (0 = off, 1 = debugging) }, { stderr, 'E', NULL, 0, Switch the log to standard error }, { foreground, 'F', NULL, 0, @@ -64,6 +64,8 @@ static struct argp_option options[] = { Bind to UDP port PORT. Default: CLD_DEF_PORT }, { pid, 'P', FILE, 0, Write daemon process id to FILE. Default: CLD_DEF_PIDFN }, + { verbose, 'v', NULL, 0, + Enable the session-level verbosity }, { strict-free, 1001, NULL, 0, For memory-checker runs. When shutting down server, free local heap, rather than simply exit(2)ing and letting OS clean up. }, As is hinted by the current code's debugging switch being an integer 'level' value, the server [and client?] has increasing levels of verbosity. The debug levels are 0: key messages affecting server operation, only 1: debugging output enabled, sans per-packet output 2: debugging output enabled, including per-packet output ie. clearly ordered by increasing value == increased verbosity. As is clearly illustrated when I cut the patch down to the above snippet, the user interface you have created gives the user two knobs for log verbosity, and it is not clear to a casual user which knob controls which sets of messages. That makes for a -more- confusing user interface, because the user must constantly ask themselves the question do I need debug? or verbose? I don't know! Additionally, this interface changes runs counter to other tools, which increase verbosity with added -v switches -- analagous to the existing integer-based debug level interface. If it is truly your desire to permit fine-grained selection of certain classes of messages, then don't dick around! Go ahead and create a bitmap log mask which permits fine-grained selection of various messages, much like netif_msg_* and netif_msg_init() in the kernel's include/linux/netdevice.h. Having two switches, -d and -v, for different, undocumented classes of message just increases confusion. Put yourself in the mind of a user trying to figure out which is which. I readily admit the __internal implementation__ resulting from your patches is a useful cleanup, but at a macro level, it merely increases logging user interface confusion. Jeff -- To unsubscribe from this list: send the line unsubscribe hail-devel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch 1/7] tabled: make two dump displays uniform
On 04/01/2010 09:51 PM, Pete Zaitcev wrote: From: Jeff Garzikjgar...@pobox.com Subject: Re: Tabled issues Date: Mon, 29 Mar 2010 15:32:33 -0400 I asserted that the standard stats dump facility must dump all available statistics. That does not exclude other methods of stat(us) dumping. Your patch added new stats to the HTML-pretty version of output, but failed to add the new stats to the standard stat dump facility. Your wish is my command. Signed-off-by: Pete Zaitcevzait...@redhat.com --- server/replica.c | 28 + server/server.c | 47 ++ server/status.c | 22 +-- server/storage.c | 50 + server/tabled.h |3 ++ 5 files changed, 117 insertions(+), 33 deletions(-) applied, thanks. I will endeavor to make the stats dump more like nfs4d in the future, FWIW. -- To unsubscribe from this list: send the line unsubscribe hail-devel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch 2/7] tabled: fix the endless recusion when reading long objects
On 04/01/2010 09:51 PM, Pete Zaitcev wrote: At certain network and disk speeds, tabled can blow its stack by filling it with (essentially) endless recursion: #2 0x0040c077 in cli_write_free (cli=value optimized out, tmp= 0x7bb910, done=value optimized out) at server.c:397 #3 0x0040ca55 in cli_writable (cli=0x686e90) at server.c:525 #4 0x0040da65 in cli_write_start (cli=0x686e90) at server.c:561 #5 0x00408ad5 in object_get_poke (cli=0x686e90) at object.c:1039 #6 0x0040c077 in cli_write_free (cli=value optimized out, tmp= 0x7bb8d0, done=value optimized out) at server.c:397 #7 0x0040ca55 in cli_writable (cli=0x686e90) at server.c:525 #8 0x0040da65 in cli_write_start (cli=0x686e90) at server.c:561 #9 0x00408ad5 in object_get_poke (cli=0x686e90) at object.c:1039 #10 0x0040c077 in cli_write_free (cli=value optimized out, tmp= 0x7bb890, done=value optimized out) at server.c:397 The fix is to deliver callbacks only from the top level. Callbacks must be delivered every time a send is completed, which amounts to every call to is_writeable(). Since there is a large number of callers to it, we found it advantageous to run callbacks from every source of events. In other words, every function that is passed to event_set must invoke cli_write_run_compl. Mind that storage.c contains calls to event_set. Signed-off-by: Pete Zaitcevzait...@redhat.com --- server/object.c |4 +++ server/server.c | 52 +++--- server/tabled.h |6 + 3 files changed, 50 insertions(+), 12 deletions(-) applied 2-7 -- To unsubscribe from this list: send the line unsubscribe hail-devel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch 1/7] tabled: make two dump displays uniform
On Tue, 06 Apr 2010 12:57:57 -0400 Jeff Garzik j...@garzik.org wrote: applied, thanks. I will endeavor to make the stats dump more like nfs4d in the future, FWIW. I was going to look into it too, but there's now some stuff I urgently need to address in Darcy's report on tabled's faults. So I only wrote nfs4 into the endless TODO. -- Pete -- To unsubscribe from this list: send the line unsubscribe hail-devel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch 1/3] CLD: End-to-end verbosity
On 04/06/2010 11:32 PM, Pete Zaitcev wrote: On Tue, 06 Apr 2010 10:40:33 -0400 Jeff Garzikj...@garzik.org wrote: The debug levels are 0: key messages affecting server operation, only 1: debugging output enabled, sans per-packet output 2: debugging output enabled, including per-packet output The previous patch did just that: Why did you reject it? That's a damned good question. I have no idea. Did I ever reply to that patch? It looks like I fscked up and missed it? Jeff -- To unsubscribe from this list: send the line unsubscribe hail-devel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch 1/3] CLD: End-to-end verbosity
On Wed, 07 Apr 2010 00:36:32 -0400 Jeff Garzik j...@garzik.org wrote: On 04/06/2010 11:32 PM, Pete Zaitcev wrote: On Tue, 06 Apr 2010 10:40:33 -0400 Jeff Garzikj...@garzik.org wrote: The debug levels are 0: key messages affecting server operation, only 1: debugging output enabled, sans per-packet output 2: debugging output enabled, including per-packet output The previous patch did just that: Why did you reject it? That's a damned good question. I have no idea. Did I ever reply to that patch? It looks like I fscked up and missed it? Apparently no reply (by e-mail): http://marc.info/?l=hail-develm=126575343714155w=2 I recall you said that users hate bitmasks on IRC or in other thread, so I thought that -D and -v would be right (effectively a bitmask but no hexadecimal at least). -- Pete -- To unsubscribe from this list: send the line unsubscribe hail-devel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html