This is an automated email from the ASF dual-hosted git repository.
jvanderzee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 77b226d74f Add tests for writing documents to the cache (#11347)
77b226d74f is described below
commit 77b226d74fc28c79a3fab6b34986e01f6ed3555c
Author: JosiahWI <[email protected]>
AuthorDate: Thu May 30 07:59:23 2024 -0500
Add tests for writing documents to the cache (#11347)
* Add tests for writing documents to the cache
This tests that the correct document data is written to cache by
aggWrite. It also covers some fields of the Doc struct used for reading
the data: the magic value, the length and header length, and the
optional checksum.
This commit also fixes the use of an uninitialized variable (header.phase)
reported by valgrind by initializing it to the same default value used
in the Stripe::_clear_init.
* Test writing an evacuated document to the cache
This tests VCs with f.evacuator set to make sure all main code paths in
agg_copy are covered. It is very similar to the test for normal VCs, but
sets the buffer up with a document instead of the raw content string.
This commit also moves FakeVC and WaitingVC into a new file called
test_doubles.h.
---
src/iocore/cache/unit_tests/test_Stripe.cc | 255 +++++++++++++++++++----------
src/iocore/cache/unit_tests/test_doubles.h | 125 ++++++++++++++
2 files changed, 292 insertions(+), 88 deletions(-)
diff --git a/src/iocore/cache/unit_tests/test_Stripe.cc
b/src/iocore/cache/unit_tests/test_Stripe.cc
index 5fe47b745b..ae5a05e9e1 100644
--- a/src/iocore/cache/unit_tests/test_Stripe.cc
+++ b/src/iocore/cache/unit_tests/test_Stripe.cc
@@ -22,12 +22,14 @@
*/
#include "main.h"
+#include "test_doubles.h"
#include "tscore/EventNotify.h"
#include <array>
+#include <cstdint>
#include <cstdio>
-#include <ostream>
+#include <cstring>
// Required by main.h
int cache_vols = 1;
@@ -79,79 +81,6 @@ std::array<AddWriterBranchTest, 32>
add_writer_branch_test_cases = {
}
};
-class FakeVC : public CacheVC
-{
-public:
- FakeVC()
- {
- this->buf = new_IOBufferData(iobuffer_size_to_index(1024,
MAX_BUFFER_SIZE_INDEX), MEMALIGNED);
- SET_HANDLER(&FakeVC::handle_call);
- }
-
- void
- set_agg_len(int agg_len)
- {
- this->agg_len = agg_len;
- }
-
- void
- set_header_len(int header_len)
- {
- this->header_len = header_len;
- }
-
- void
- set_write_len(int write_len)
- {
- this->write_len = write_len;
- }
-
- void
- set_readers(int readers)
- {
- this->f.readers = readers;
- }
-
- int
- handle_call(int /* event ATS_UNUSED */, void * /* e ATS_UNUSED */)
- {
- return EVENT_CONT;
- }
-};
-
-class WaitingVC final : public FakeVC
-{
-public:
- WaitingVC(Stripe *stripe)
- {
- SET_HANDLER(&WaitingVC::handle_call);
- this->stripe = stripe;
- this->dir = *stripe->dir;
- }
-
- void
- wait_for_callback()
- {
- this->_notifier.lock();
- while (!this->_got_callback) {
- this->_notifier.wait();
- }
- this->_notifier.unlock();
- }
-
- int
- handle_call(int /* event ATS_UNUSED */, void * /* e ATS_UNUSED */)
- {
- this->_got_callback = true;
- this->_notifier.signal();
- return EVENT_CONT;
- }
-
-private:
- EventNotify _notifier;
- bool _got_callback{false};
-};
-
/* Catch test helper to provide a Stripe with a valid file descriptor.
*
* The file will be deleted automatically when the application ends normally.
@@ -175,12 +104,14 @@ attach_tmpfile_to_stripe(Stripe &stripe)
// We can't return a stripe from this function because the copy
// and move constructors are deleted.
-static void
+static std::FILE *
init_stripe_for_writing(Stripe &stripe, StripteHeaderFooter &header, CacheVol
&cache_vol)
{
stripe.cache_vol = &cache_vol;
cache_rsb.write_backlog_failure =
Metrics::Counter::createPtr("unit_test.write.backlog.failure");
stripe.cache_vol->vol_rsb.write_backlog_failure =
Metrics::Counter::createPtr("unit_test.write.backlog.failure");
+ cache_rsb.gc_frags_evacuated =
Metrics::Counter::createPtr("unit_test.gc.frags.evacuated");
+ stripe.cache_vol->vol_rsb.gc_frags_evacuated =
Metrics::Counter::createPtr("unit_test.gc.frags.evacuated");
// A number of things must be initialized in a certain way for Stripe
// not to segfault, hit an assertion, or exhibit zero-division.
@@ -199,8 +130,9 @@ init_stripe_for_writing(Stripe &stripe, StripteHeaderFooter
&header, CacheVol &c
header.write_pos = 50000;
header.agg_pos = 1;
+ header.phase = 0;
stripe.header = &header;
- attach_tmpfile_to_stripe(stripe);
+ return attach_tmpfile_to_stripe(stripe);
}
TEST_CASE("The behavior of Stripe::add_writer.")
@@ -260,14 +192,16 @@ TEST_CASE("The behavior of Stripe::add_writer.")
// This test case demonstrates how to set up a Stripe and make
// a call to aggWrite without causing memory errors. It uses a
// tmpfile for the Stripe to write to.
-TEST_CASE("aggWrite behavior")
+TEST_CASE("aggWrite behavior with f.evacuator unset")
{
Stripe stripe;
StripteHeaderFooter header;
CacheVol cache_vol;
- init_stripe_for_writing(stripe, header, cache_vol);
- WaitingVC vc{&stripe};
- vc.set_write_len(1);
+ auto *file{init_stripe_for_writing(stripe, header, cache_vol)};
+ WaitingVC vc{&stripe};
+ char const *source = "yay";
+ vc.set_test_data(source, 4);
+ vc.set_write_len(4);
vc.set_agg_len(stripe.round_to_approx_size(vc.write_len + vc.header_len +
vc.frag_len));
stripe.add_writer(&vc);
@@ -284,26 +218,171 @@ TEST_CASE("aggWrite behavior")
CHECK(0 == header.agg_pos);
}
- SECTION("Given the aggregation buffer is partially full and sync is set, "
- "when we schedule aggWrite, "
- "then some bytes should be written to disk.")
+ SECTION("Given the aggregation buffer is partially full, sync is set, "
+ "and checksums are enabled, "
+ "when we schedule aggWrite with a VC buffer containing 'yay', ")
+ {
+ cache_config_enable_checksum = true;
+ vc.f.sync = 1;
+ vc.f.use_first_key = 1;
+ vc.write_serial = 1;
+ header.write_serial = 10;
+ int document_offset = header.write_pos;
+ {
+ SCOPED_MUTEX_LOCK(lock, stripe.mutex, this_ethread());
+ stripe.aggWrite(EVENT_NONE, 0);
+ }
+ vc.wait_for_callback();
+
+ SECTION("then some bytes should be written to disk")
+ {
+ CHECK(0 < header.agg_pos);
+ }
+
+ Doc doc;
+
+ {
+ // Other threads are still alive and may interact with the
+ // cache periodically, so we should always grab the lock
+ // whenever we interact with the underlying file.
+ SCOPED_MUTEX_LOCK(lock, stripe.mutex, this_ethread());
+ fseek(file, document_offset, SEEK_SET);
+ fread(&doc, sizeof(Doc), 1, file);
+ }
+
+ // An incorrect magic value would indicate that the document is corrupt.
+ REQUIRE(DOC_MAGIC == doc.magic);
+
+ SECTION("then the document should be single fragment.")
+ {
+ CHECK(doc.single_fragment());
+ }
+
+ SECTION("then the document header length should be 0.")
+ {
+ CHECK(0 == doc.hlen);
+ }
+
+ SECTION("then the document length should be sizeof(Doc) + 4.")
+ {
+ CHECK(sizeof(Doc) + 4 == doc.len);
+ }
+
+ SECTION("then the document checksum should be correct.")
+ {
+ std::uint32_t expected = 'y' + 'a' + 'y' + '\0';
+ CHECK(expected == doc.checksum);
+ }
+
+ SECTION("then the document data should contain 'yay'.")
+ {
+ char *data = new char[doc.data_len()];
+ {
+ SCOPED_MUTEX_LOCK(lock, stripe.mutex, this_ethread());
+ fseek(file, document_offset + doc.prefix_len(), SEEK_SET);
+ fread(data, doc.data_len(), 1, file);
+ }
+ INFO("buffer content is \"" << data << "\"");
+ CHECK(0 == strncmp("yay", data, 3));
+ delete[] data;
+ }
+
+ cache_config_enable_checksum = false;
+ }
+
+ ats_free(stripe.raw_dir);
+ ats_free(stripe.evacuate);
+}
+
+// When f.evacuator is set, vc.buf must contain a Doc object including headers
+// and data.
+// We don't use a CacheEvacuateDocVC because the behavior under test depends
+// only on the presence of the f.evacuator flag.
+TEST_CASE("aggWrite behavior with f.evacuator set")
+{
+ Stripe stripe;
+ StripteHeaderFooter header;
+ CacheVol cache_vol;
+ auto *file{init_stripe_for_writing(stripe, header, cache_vol)};
+ WaitingVC vc{&stripe};
+ char *source = new char[sizeof(Doc) + 4]{};
+ const char *yay = "yay";
+ Doc doc{};
+ doc.magic = DOC_MAGIC;
+ doc.len = sizeof(Doc) + 4;
+ doc.total_len = 4;
+ doc.hlen = 0;
+ std::memcpy(source, &doc, sizeof(doc));
+ std::memcpy(source + sizeof(Doc), yay, 4);
+ vc.set_test_data(source, sizeof(Doc) + 4);
+ vc.set_write_len(stripe.round_to_approx_size(doc.len));
+ vc.set_agg_len(stripe.round_to_approx_size(vc.write_len + vc.header_len +
vc.frag_len));
+ vc.mark_as_evacuator();
+ stripe.add_writer(&vc);
+
+ SECTION("Given the aggregation buffer is partially full, sync is set, "
+ "when we schedule aggWrite with a VC buffer containing 'yay', ")
{
vc.f.sync = 1;
vc.f.use_first_key = 1;
vc.write_serial = 1;
header.write_serial = 10;
+ int document_offset = header.write_pos;
{
SCOPED_MUTEX_LOCK(lock, stripe.mutex, this_ethread());
stripe.aggWrite(EVENT_NONE, 0);
}
vc.wait_for_callback();
- // We don't check here what bytes were written. In fact according
- // to valgrind it's writing uninitialized parts of the aggregation
- // buffer, but that's OK because in this SECTION we only care that
- // something was written successfully without anything blowing up.
- CHECK(0 < header.agg_pos);
+
+ SECTION("then some bytes should be written to disk")
+ {
+ CHECK(0 < header.agg_pos);
+ }
+
+ Doc doc;
+
+ {
+ // Other threads are still alive and may interact with the
+ // cache periodically, so we should always grab the lock
+ // whenever we interact with the underlying file.
+ SCOPED_MUTEX_LOCK(lock, stripe.mutex, this_ethread());
+ fseek(file, document_offset, SEEK_SET);
+ fread(&doc, sizeof(Doc), 1, file);
+ }
+
+ // An incorrect magic value would indicate that the document is corrupt.
+ REQUIRE(DOC_MAGIC == doc.magic);
+
+ SECTION("then the document should be single fragment.")
+ {
+ CHECK(doc.single_fragment());
+ }
+
+ SECTION("then the document header length should be 0.")
+ {
+ CHECK(0 == doc.hlen);
+ }
+
+ SECTION("then the document length should be sizeof(Doc) + 4.")
+ {
+ CHECK(sizeof(Doc) + 4 == doc.len);
+ }
+
+ SECTION("then the document data should contain 'yay'.")
+ {
+ char *data = new char[doc.data_len()];
+ {
+ SCOPED_MUTEX_LOCK(lock, stripe.mutex, this_ethread());
+ fseek(file, document_offset + doc.prefix_len(), SEEK_SET);
+ fread(data, doc.data_len(), 1, file);
+ }
+ INFO("buffer content is \"" << data << "\"");
+ CHECK(0 == strncmp("yay", data, 3));
+ delete[] data;
+ }
}
+ delete[] source;
ats_free(stripe.raw_dir);
ats_free(stripe.evacuate);
}
diff --git a/src/iocore/cache/unit_tests/test_doubles.h
b/src/iocore/cache/unit_tests/test_doubles.h
new file mode 100644
index 0000000000..3ab4e1f8cd
--- /dev/null
+++ b/src/iocore/cache/unit_tests/test_doubles.h
@@ -0,0 +1,125 @@
+/** @file
+
+ A brief file description
+
+ @section license License
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+ */
+
+#pragma once
+
+#include "main.h"
+
+#include "tscore/EventNotify.h"
+
+#include <cstdint>
+#include <cstring>
+#include <stdexcept>
+
+class FakeVC : public CacheVC
+{
+public:
+ FakeVC()
+ {
+ this->buf = new_IOBufferData(iobuffer_size_to_index(1024,
MAX_BUFFER_SIZE_INDEX), MEMALIGNED);
+ this->blocks = new_IOBufferBlock();
+ this->blocks->set(this->buf.get());
+ SET_HANDLER(&FakeVC::handle_call);
+ }
+
+ void
+ set_test_data(char const *source, int len)
+ {
+ this->blocks->reset();
+ if (len > 1024) {
+ throw std::runtime_error{"data length exceeds internal buffer"};
+ }
+ std::memcpy(this->buf->data(), source, len);
+ this->blocks->fill(len);
+ }
+
+ void
+ set_agg_len(int agg_len)
+ {
+ this->agg_len = agg_len;
+ }
+
+ void
+ set_header_len(int header_len)
+ {
+ this->header_len = header_len;
+ }
+
+ void
+ set_write_len(int write_len)
+ {
+ this->write_len = write_len;
+ this->total_len = static_cast<std::uint64_t>(write_len);
+ }
+
+ void
+ set_readers(int readers)
+ {
+ this->f.readers = readers;
+ }
+
+ void
+ mark_as_evacuator()
+ {
+ this->f.evacuator = true;
+ }
+
+ int
+ handle_call(int /* event ATS_UNUSED */, void * /* e ATS_UNUSED */)
+ {
+ return EVENT_CONT;
+ }
+};
+
+class WaitingVC final : public FakeVC
+{
+public:
+ WaitingVC(Stripe *stripe)
+ {
+ SET_HANDLER(&WaitingVC::handle_call);
+ this->stripe = stripe;
+ this->dir = *stripe->dir;
+ }
+
+ void
+ wait_for_callback()
+ {
+ this->_notifier.lock();
+ while (!this->_got_callback) {
+ this->_notifier.wait();
+ }
+ this->_notifier.unlock();
+ }
+
+ int
+ handle_call(int /* event ATS_UNUSED */, void * /* e ATS_UNUSED */)
+ {
+ this->_got_callback = true;
+ this->_notifier.signal();
+ return EVENT_CONT;
+ }
+
+private:
+ EventNotify _notifier;
+ bool _got_callback{false};
+};
