[PATCH] ctdb-test: Add tests to verify queue processing
swen
swen at linux.ibm.com
Thu Mar 21 10:05:29 UTC 2019
Hi Martin
On Thu, 2019-03-21 at 17:15 +1100, Martin Schwenke wrote:
> Hi Swen,
>
> On Mon, 18 Mar 2019 15:34:30 +0100, swen <swen at linux.ibm.com> wrote:
>
> > On Mon, 2019-03-18 at 21:58 +1100, Martin Schwenke wrote:
> > > 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...
> > 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.
>
> I would suggest not changing test_setup() to *return* the queue,
> while
> dropping the ctdb output parameter. That forces the queue it into
> tests
> that don't use it.
>
> Instead, as I suggested in my previous email, I suggest adding an
> optional output argument for the queue and only pass it out if non-
> NULL
> is passed in. That way, the change to existing tests is minimal:
> just
> pass NULL for the extra argument. This makes it clear that those
> tests
> aren't looking inside the queue.
>
> The new tests could then follow the existing pattern of using ctdb-
> >ev
> for the event loop and using queue when necessary.
>
> How does that sound?
Well, I would have preferred to "use" ctdb_io as it is by the real
functional consumers in the code, meaning, to have the queue as the
center of gravity.
But I do see your point.
Therefore, I changed it to the way you prefer.
Please review and push if happy
... you shoudld be totally excited now :-)
Thanks for your support in advance.
Cheers Swen
P.S.: One thing I have to say.... I really, really dislike the
hungarian notation, totally odd stuff, and I felt physical pain to not
change it and even worse to add to it.
-------------- next part --------------
From 12a54f9bb358d71920346f5874f5a23fa67910df 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 additional parameter.
Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
ctdb/tests/src/ctdb_io_test.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/ctdb/tests/src/ctdb_io_test.c b/ctdb/tests/src/ctdb_io_test.c
index 5a2f81538a1..668b1f0afe5 100644
--- a/ctdb/tests/src/ctdb_io_test.c
+++ b/ctdb/tests/src/ctdb_io_test.c
@@ -34,7 +34,8 @@ void ctdb_set_error(struct ctdb_context *ctdb, const char *fmt, ...)
static void test_setup(ctdb_queue_cb_fn_t cb,
int *pfd,
- struct ctdb_context **pctdb)
+ struct ctdb_context **pctdb,
+ struct ctdb_queue **pqueue)
{
int pipefd[2], ret;
struct ctdb_context *ctdb;
@@ -54,6 +55,9 @@ static void test_setup(ctdb_queue_cb_fn_t cb,
*pctdb = ctdb;
*pfd = pipefd[1];
+ if (pqueue != NULL) {
+ *pqueue = queue;
+ }
}
static const size_t test1_req_len = 8;
@@ -76,7 +80,7 @@ static void test1(void)
int fd, ret;
uint32_t pkt_size;
- test_setup(test1_callback, &fd, &ctdb);
+ test_setup(test1_callback, &fd, &ctdb, NULL);
pkt_size = sizeof(uint32_t) + test1_req_len;
ret = write(fd, &pkt_size, sizeof(pkt_size));
@@ -116,7 +120,7 @@ static void test2(void)
req[i] = i % CHAR_MAX;
}
- test_setup(test2_callback, &fd, &ctdb);
+ test_setup(test2_callback, &fd, &ctdb, NULL);
/*
* request 0
--
2.20.1
From fc02d569528308adbc619d17efe05fba26e41481 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 | 76 +++++++++++++++++++++++++++++++++++
1 file changed, 76 insertions(+)
diff --git a/ctdb/tests/src/ctdb_io_test.c b/ctdb/tests/src/ctdb_io_test.c
index 668b1f0afe5..ec1d25859f0 100644
--- a/ctdb/tests/src/ctdb_io_test.c
+++ b/ctdb/tests/src/ctdb_io_test.c
@@ -172,6 +172,78 @@ static void test2(void)
TALLOC_FREE(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_context *ctdb;
+ struct ctdb_queue *queue;
+ uint32_t pkt_size;
+ char *request;
+ size_t req_len;
+ int fd;
+ int ret;
+
+ test_setup(test_cb, &fd, &ctdb, &queue);
+ 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(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(ctdb->ev);
+ /*
+ * needs to be called twice as an incomplete packet
+ * does not trigger a schedule_immediate
+ */
+ tevent_loop_once(ctdb->ev);
+
+ assert(queue->buffer.offset == 0);
+
+ TALLOC_FREE(ctdb);
+}
+
+
int main(int argc, const char **argv)
{
int num;
@@ -192,6 +264,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 1552f02f7a1cc9c8b2fa48cf2c18a35b473a45ae 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 | 74 +++++++++++++++++++++++++++++++++++
1 file changed, 74 insertions(+)
diff --git a/ctdb/tests/src/ctdb_io_test.c b/ctdb/tests/src/ctdb_io_test.c
index ec1d25859f0..9cd02aa0eaa 100644
--- a/ctdb/tests/src/ctdb_io_test.c
+++ b/ctdb/tests/src/ctdb_io_test.c
@@ -243,6 +243,76 @@ static void test3(void)
TALLOC_FREE(ctdb);
}
+static void test4(void)
+{
+ struct ctdb_context *ctdb;
+ struct ctdb_queue *queue;
+ uint32_t pkt_size;
+ char *request;
+ size_t req_len;
+ int fd;
+ int ret;
+
+ test_setup(test_cb, &fd, &ctdb, &queue);
+
+ 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(ctdb->ev);
+ /*
+ * needs to be called twice as an initial incomplete packet
+ * does not trigger a schedule_immediate
+ */
+ tevent_loop_once(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(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(ctdb->ev);
+
+ /* back to standard buffer size */
+ assert(queue->buffer.size == queue->buffer_size);
+
+ TALLOC_FREE(ctdb);
+}
int main(int argc, const char **argv)
{
@@ -268,6 +338,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 d1903c9368e5045bbcf8b492bf7790392892417d 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/20190321/2c518408/signature.sig>
More information about the samba-technical
mailing list