[PATCH] ctdb-test: Add tests to verify queue processing
swen
swen at linux.ibm.com
Mon Mar 18 14:34:30 UTC 2019
Hi Martin
Thanks for your reply.
I integrated the patches into ctdb_io_test.c and performed all the
other updates as well (ctdb_io_test_001.sh)
Please review and push if happy.
Thanks for your support in advance.
Cheers Swen
On Mon, 2019-03-18 at 21:58 +1100, Martin Schwenke wrote:
> Hi Swen,
>
> On Thu, 14 Mar 2019 13:27:21 +0100, swen <swen at linux.ibm.com> wrote:
>
> > On Thu, 2019-03-14 at 21:56 +1100, Martin Schwenke via samba-
> > technical
> > wrote:
> > > On Thu, 07 Mar 2019 13:03:15 +0100, swen <swen at linux.ibm.com>
> > > wrote:
> > >
> > > > Followed Christof's good example and created a test verifying
> > > > the correct processing of CTDB's queue buffer.
> > >
> > > This looks like it might fit nicely into the existing
> > > ctdb/tests/src/ctdb_io_test.c instead of creating a new test
> > > program.
> > Hmm, well it actually doesn't fit nicely :-)
> >
> > The existing tests do investigate what's received on "client" side
> > and
> > therefore, do not check or need anything on the servers side.
> >
> > The tests I'm adding are actually covering the server side and
> > investigate the queue itself and the byte-pushing there... not
> > care'ing
> > what's received.
> >
> > Due to this difference the existing tests have no reference to the
> > queue which is needed fo the tests I would like t add.
> >
> > Aynhow, if you'd prefer to just have on file containing the tests,
> > I guess, I could do that. Just thought it would be nicer to have
> > the
> > separation because of what I described.
> >
> > You pick, I don't mind.
>
> I think the 2 infrastructure functions (ctdb_set_error() and
> test_setup()) are similar enough that we don't need another file with
> them essentially duplicated. I would just add a struct ctdb_queue **
> argument to test_setup() and set that if non-NULL is passed in. Then
> you're only really adding a couple of testcases...
>
> peace & happiness,
> martin
-------------- next part --------------
From 33d7df8fd869ae5428f46b8c2edb8ff75a1f483e Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Mon, 18 Mar 2019 15:15:25 +0100
Subject: [PATCH 1/4] ctdb-test: Modify ctdb_io_test test_setup to provide
queue reference
Some test scenarios require access to the created queue.
Prepare the test_setup function to provide it as its return value.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
ctdb/tests/src/ctdb_io_test.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/ctdb/tests/src/ctdb_io_test.c b/ctdb/tests/src/ctdb_io_test.c
index 5a2f81538a1..d1173f7fa47 100644
--- a/ctdb/tests/src/ctdb_io_test.c
+++ b/ctdb/tests/src/ctdb_io_test.c
@@ -32,9 +32,7 @@ void ctdb_set_error(struct ctdb_context *ctdb, const char *fmt, ...)
assert(false);
}
-static void test_setup(ctdb_queue_cb_fn_t cb,
- int *pfd,
- struct ctdb_context **pctdb)
+static struct ctdb_queue *test_setup(ctdb_queue_cb_fn_t cb, int *fd)
{
int pipefd[2], ret;
struct ctdb_context *ctdb;
@@ -52,8 +50,8 @@ static void test_setup(ctdb_queue_cb_fn_t cb,
NULL, "test queue");
assert(queue != NULL);
- *pctdb = ctdb;
- *pfd = pipefd[1];
+ *fd = pipefd[1];
+ return queue;
}
static const size_t test1_req_len = 8;
@@ -72,11 +70,11 @@ static void test1_callback(uint8_t *data, size_t length, void *private_data)
static void test1(void)
{
- struct ctdb_context *ctdb;
+ struct ctdb_queue *queue;
int fd, ret;
uint32_t pkt_size;
- test_setup(test1_callback, &fd, &ctdb);
+ queue = test_setup(test1_callback, &fd);
pkt_size = sizeof(uint32_t) + test1_req_len;
ret = write(fd, &pkt_size, sizeof(pkt_size));
@@ -85,9 +83,9 @@ static void test1(void)
ret = write(fd, test1_req, test1_req_len);
assert(ret == test1_req_len);
- tevent_loop_once(ctdb->ev);
+ tevent_loop_once(queue->ctdb->ev);
- TALLOC_FREE(ctdb);
+ TALLOC_FREE(queue->ctdb);
}
static const size_t test2_req_len[] = { 900, 24, 600 };
@@ -107,7 +105,7 @@ static void test2_callback(uint8_t *data, size_t length, void *private_data)
static void test2(void)
{
- struct ctdb_context *ctdb;
+ struct ctdb_queue *queue;
int fd, ret, i;
uint32_t pkt_size;
char req[1024] = { 0 };
@@ -116,7 +114,7 @@ static void test2(void)
req[i] = i % CHAR_MAX;
}
- test_setup(test2_callback, &fd, &ctdb);
+ queue = test_setup(test2_callback, &fd);
/*
* request 0
@@ -142,7 +140,7 @@ static void test2(void)
ret = write(fd, req, test2_req_len[1] - 1);
assert(ret == test2_req_len[1] - 1);
- tevent_loop_once(ctdb->ev);
+ tevent_loop_once(queue->ctdb->ev);
/*
* Write the missing byte now.
@@ -160,12 +158,12 @@ static void test2(void)
ret = write(fd, req, test2_req_len[2]);
assert(ret == test2_req_len[2]);
- tevent_loop_once(ctdb->ev);
- tevent_loop_once(ctdb->ev);
+ tevent_loop_once(queue->ctdb->ev);
+ tevent_loop_once(queue->ctdb->ev);
assert(test2_cb_num == 2);
- TALLOC_FREE(ctdb);
+ TALLOC_FREE(queue->ctdb);
}
int main(int argc, const char **argv)
--
2.20.1
From f1991f3df0bc2dcfbd6d0d99212e35c1e262925c Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Mon, 18 Mar 2019 15:22:19 +0100
Subject: [PATCH 2/4] ctdb-test: Adding test case verifying data in buffer move
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
ctdb/tests/src/ctdb_io_test.c | 75 +++++++++++++++++++++++++++++++++++
1 file changed, 75 insertions(+)
diff --git a/ctdb/tests/src/ctdb_io_test.c b/ctdb/tests/src/ctdb_io_test.c
index d1173f7fa47..9dd92612df3 100644
--- a/ctdb/tests/src/ctdb_io_test.c
+++ b/ctdb/tests/src/ctdb_io_test.c
@@ -166,6 +166,77 @@ static void test2(void)
TALLOC_FREE(queue->ctdb);
}
+static void test_cb(uint8_t *data, size_t length, void *private_data)
+{
+ /* dummy handler, not verifying anything */
+ TALLOC_FREE(data);
+}
+
+static void test3(void)
+{
+ struct ctdb_queue *queue;
+ uint32_t pkt_size;
+ char *request;
+ size_t req_len;
+ int fd;
+ int ret;
+
+ queue = test_setup(test_cb, &fd);
+ request = talloc_zero_size(queue, queue->buffer_size);
+
+ /*
+ * calculate a request length which will fit into the buffer
+ * but not twice. Because we need to write the size integer
+ * as well (4-bytes) we're guaranteed that no 2 packets will fit.
+ */
+ req_len = queue->buffer_size >> 1;
+
+ /* writing first packet */
+ pkt_size = sizeof(uint32_t) + req_len;
+
+ ret = write(fd, &pkt_size, sizeof(pkt_size));
+ assert(ret == sizeof(pkt_size));
+
+ ret = write(fd, request, req_len);
+ assert(ret == req_len);
+
+ /* writing second, incomplete packet */
+ pkt_size = sizeof(uint32_t) + req_len;
+
+ ret = write(fd, &pkt_size, sizeof(pkt_size));
+ assert(ret == sizeof(pkt_size));
+
+ ret = write(fd, request, req_len >> 1);
+ assert(ret == req_len >> 1);
+
+ /* process...only 1st packet can be processed */
+ tevent_loop_once(queue->ctdb->ev);
+
+ /* we should see a progressed offset of req_len + sizeof(pkt_size) */
+ assert(queue->buffer.offset == req_len + sizeof(pkt_size));
+
+ /* writing another few bytes of the still incomplete packet */
+ ret = write(fd, request, (req_len >> 1) - 1);
+ assert(ret == (req_len >> 1) - 1);
+
+ /*
+ * the packet is still incomplete and connot be processed
+ * but the packet data had to be moved in the buffer in order
+ * to fetch the new 199 bytes -> offset must be 0 now.
+ */
+ tevent_loop_once(queue->ctdb->ev);
+ /*
+ * needs to be called twice as an incomplete packet
+ * does not trigger a schedule_immediate
+ */
+ tevent_loop_once(queue->ctdb->ev);
+
+ assert(queue->buffer.offset == 0);
+
+ TALLOC_FREE(queue->ctdb);
+}
+
+
int main(int argc, const char **argv)
{
int num;
@@ -186,6 +257,10 @@ int main(int argc, const char **argv)
test2();
break;
+ case 3:
+ test3();
+ break;
+
default:
fprintf(stderr, "Unknown test number %s\n", argv[1]);
}
--
2.20.1
From 740ac7d740aeff4f5afc9fe2b9188dc02f4efb5f Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Mon, 18 Mar 2019 15:25:54 +0100
Subject: [PATCH 3/4] ctdb-test: Adding test case to verify queue resizeing
If a data packet arrives which exceeds the queue's current buffer size,
the buffer needs to be increased to hold the full packet. Once the poacket
is processed the buffer size should be decreased to its standard size again.
This test case verifies this process.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
ctdb/tests/src/ctdb_io_test.c | 73 +++++++++++++++++++++++++++++++++++
1 file changed, 73 insertions(+)
diff --git a/ctdb/tests/src/ctdb_io_test.c b/ctdb/tests/src/ctdb_io_test.c
index 9dd92612df3..cf4aa339b5e 100644
--- a/ctdb/tests/src/ctdb_io_test.c
+++ b/ctdb/tests/src/ctdb_io_test.c
@@ -236,6 +236,75 @@ static void test3(void)
TALLOC_FREE(queue->ctdb);
}
+static void test4(void)
+{
+ struct ctdb_queue *queue;
+ uint32_t pkt_size;
+ char *request;
+ size_t req_len;
+ int fd;
+ int ret;
+
+ queue = test_setup(test_cb, &fd);
+
+ req_len = queue->buffer_size << 1; /* double the buffer size */
+ request = talloc_zero_size(queue, req_len);
+
+ /* writing first part of packet exceeding standard buffer size */
+ pkt_size = sizeof(uint32_t) + req_len;
+
+ ret = write(fd, &pkt_size, sizeof(pkt_size));
+ assert(ret == sizeof(pkt_size));
+
+ ret = write(fd, request, req_len - (queue->buffer_size >> 1));
+ assert(ret == req_len - (queue->buffer_size >> 1));
+
+ /*
+ * process...
+ * this needs to be done to have things changed
+ */
+ tevent_loop_once(queue->ctdb->ev);
+ /*
+ * needs to be called twice as an initial incomplete packet
+ * does not trigger a schedule_immediate
+ */
+ tevent_loop_once(queue->ctdb->ev);
+
+ /* the buffer should be resized to packet size now */
+ assert(queue->buffer.size == pkt_size);
+
+ /* writing remaining data */
+ ret = write(fd, request, queue->buffer_size >> 1);
+ assert(ret == (queue->buffer_size >> 1));
+
+ /* process... */
+ tevent_loop_once(queue->ctdb->ev);
+
+ /*
+ * the buffer was increased beyond its standard size.
+ * once packet got processed, the buffer has to be free'd
+ * and will be re-allocated with standard size on new request arrival.
+ */
+
+ assert(queue->buffer.size == 0);
+
+ /* writing new packet to verify standard buffer size */
+ pkt_size = sizeof(uint32_t) + (queue->buffer_size >> 1);
+
+ ret = write(fd, &pkt_size, sizeof(pkt_size));
+ assert(ret == sizeof(pkt_size));
+
+ ret = write(fd, request, (queue->buffer_size >> 1));
+ assert(ret == (queue->buffer_size >> 1));
+
+ /* process... */
+ tevent_loop_once(queue->ctdb->ev);
+
+ /* back to standard buffer size */
+ assert(queue->buffer.size == queue->buffer_size);
+
+ TALLOC_FREE(queue->ctdb);
+}
int main(int argc, const char **argv)
{
@@ -261,6 +330,10 @@ int main(int argc, const char **argv)
test3();
break;
+ case 4:
+ test4();
+ break;
+
default:
fprintf(stderr, "Unknown test number %s\n", argv[1]);
}
--
2.20.1
From 09c84299814d013eefab0b9f6decbcb5fe11440c Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Mon, 18 Mar 2019 15:30:42 +0100
Subject: [PATCH 4/4] ctdb-test: Enable new ctdb_io_tests 3 and 4
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
ctdb/tests/cunit/ctdb_io_test_001.sh | 2 ++
1 file changed, 2 insertions(+)
diff --git a/ctdb/tests/cunit/ctdb_io_test_001.sh b/ctdb/tests/cunit/ctdb_io_test_001.sh
index 15842ea8e17..b6d3bce4782 100755
--- a/ctdb/tests/cunit/ctdb_io_test_001.sh
+++ b/ctdb/tests/cunit/ctdb_io_test_001.sh
@@ -6,3 +6,5 @@ ok_null
unit_test ctdb_io_test 1
unit_test ctdb_io_test 2
+unit_test ctdb_io_test 3
+unit_test ctdb_io_test 4
--
2.20.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20190318/e6fb56df/signature.sig>
More information about the samba-technical
mailing list