Skip to contentSkip to conversation

tabled: fix the endless recusion when reading long objects

PATCH Applied Pete Zaitcev 2 participants 2 messages Apr 2, 2010 – Apr 6, 2010

lore
Share & tools
lore

Pete Zaitcev

author
#0 lore raw

PATCH 2/7 tabled: fix the endless recusion when reading long objects

At certain network and disk speeds, tabled can blow its stack by filling it with (essentially) endless recursion:

#2 0x000000000040c077 in cli_write_free (cli=<value optimized out>, tmp=

    0x7bb910, done=<value optimized out>) at server.c:397
#3  0x000000000040ca55 in cli_writable (cli=0x686e90) at server.c:525
#4  0x000000000040da65 in cli_write_start (cli=0x686e90) at server.c:561
#5  0x0000000000408ad5 in object_get_poke (cli=0x686e90) at object.c:1039
#6  0x000000000040c077 in cli_write_free (cli=<value optimized out>, tmp=
    0x7bb8d0, done=<value optimized out>) at server.c:397
#7  0x000000000040ca55 in cli_writable (cli=0x686e90) at server.c:525
#8  0x000000000040da65 in cli_write_start (cli=0x686e90) at server.c:561
#9  0x0000000000408ad5 in object_get_poke (cli=0x686e90) at object.c:1039
#10 0x000000000040c077 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 Zaitcev <[email protected]>
---
 server/object.c |    4 +++
 server/server.c |   52 +++++++++++++++++++++++++++++++++++-----------
 server/tabled.h |    6 +++++
 3 files changed, 50 insertions(+), 12 deletions(-)

commit 88959fb8c4cc8232f92805618c5bcaeee9099390

Author: Pete Zaitcev <[email protected]>
Date:   Thu Apr 1 19:04:13 2010 -0600

Fix the endless recursion when reading long objects.

server/object.c +4 -0
index d61a446..6d32316 100644
@@ -994,6 +994,9 @@ static bool object_get_poke(struct client *cli)
 	char *buf;
 	ssize_t bytes;
 
+	/* XXX flow control - what treshold? */
+	/*  if (cli_wqueued(cli) >= 1000000000) return false; */
+
 	buf = malloc(CLI_DATA_BUF_SZ);
 	if (!buf)
 		return false;

@@ -1052,6 +1055,7 @@ static bool object_get_more(struct client *cli, void *cb_data, bool done)
 static void object_get_event(struct open_chunk *ochunk)
 {
 	object_get_poke(ochunk->cli);
+	cli_write_run_compl(ochunk->cli);
 }
 
 bool object_get_body(struct client *cli, const char *user, const char *bucket,
server/server.c +40 -12

129 lines not on this page · raw message

server/tabled.h +6 -0
index b4f51ed..3ef4a49 100644
@@ -164,8 +164,11 @@ struct client {
 	struct event		write_ev;
 
 	struct list_head	write_q;	/* list of async writes */
+	struct list_head	write_compl_q;	/* list of done writes */
 	size_t			write_cnt;	/* water level */
 	bool			writing;
+	/* some debugging stats */
+	size_t			write_cnt_max;
 
 	unsigned int		req_used;	/* amount of req_buf in use */
 	char			*req_ptr;	/* start of unexamined data */

@@ -212,6 +215,8 @@ struct server_stats {
 	unsigned long		event;		/* events dispatched */
 	unsigned long		tcp_accept;	/* TCP accepted cxns */
 	unsigned long		opt_write;	/* optimistic writes */
+
+	unsigned long		max_write_buf;
 };
 
 struct listen_cfg {

@@ -325,6 +330,7 @@ extern int cli_writeq(struct client *cli, const void *buf, unsigned int buflen,
 extern size_t cli_wqueued(struct client *cli);
 extern bool cli_cb_free(struct client *cli, void *cb_data, bool done);
 extern bool cli_write_start(struct client *cli);
+extern bool cli_write_run_compl(struct client *cli);
 extern int cli_req_avail(struct client *cli);
 extern void applog(int prio, const char *fmt, ...);
 extern int stor_update_cb(void);
#1 lore raw
Quoted 26 lines · Pete Zaitcev

At certain network and disk speeds, tabled can blow its stack by filling it with (essentially) endless recursion:

#2 0x000000000040c077 in cli_write_free (cli=<value optimized out>, tmp=

     0x7bb910, done=<value optimized out>) at server.c:397
#3  0x000000000040ca55 in cli_writable (cli=0x686e90) at server.c:525
#4  0x000000000040da65 in cli_write_start (cli=0x686e90) at server.c:561
#5  0x0000000000408ad5 in object_get_poke (cli=0x686e90) at object.c:1039
#6  0x000000000040c077 in cli_write_free (cli=<value optimized out>, tmp=
     0x7bb8d0, done=<value optimized out>) at server.c:397
#7  0x000000000040ca55 in cli_writable (cli=0x686e90) at server.c:525
#8  0x000000000040da65 in cli_write_start (cli=0x686e90) at server.c:561
#9  0x0000000000408ad5 in object_get_poke (cli=0x686e90) at object.c:1039
#10 0x000000000040c077 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.

Quoted 7 lines
Signed-off-by Pete Zaitcev<[email protected]>
---
  server/object.c |    4 +++
  server/server.c |   52 +++++++++++++++++++++++++++++++++++-----------
  server/tabled.h |    6 +++++
  3 files changed, 50 insertions(+), 12 deletions(-)

applied 2-7