Skip to content

Commit

Permalink
add serde index state test
Browse files Browse the repository at this point in the history
Regarding the test `serde_test` in `src/v/storage/tests/index_state_test.cc`

$> ninja storage_log_index_rpunit

$> bin/storage_log_index_rpunit
Running 7 test cases...
AddressSanitizer:DEADLYSIGNAL
=================================================================
==934285==ERROR: AddressSanitizer: stack-overflow on address 0x7fff2a209f58 (pc 0x558e9c2eab0e bp 0x7fff2a20a7a0 sp 0x7fff2a209f60 T0)
    #0 0x558e9c2eab0e in __asan_memmove /home/nwatkins/src/redpanda/vbuild/llvm/src/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:30:3
    #1 0x558e9c448ec2 in std::__1::enable_if<(is_same<std::__1::remove_const<char const>::type, char>::value) && (is_trivially_copy_assignable<char>::value), char*>::type std::__1::__copy<char const, char>(char const*, char const*, char*) /home/nwatkins/src/redpanda/vbuild/llvm/in
stall/bin/../include/c++/v1/algorithm:1735:9
    #2 0x558e9c448e1e in char* std::__1::copy<char const*, char*>(char const*, char const*, char*) /home/nwatkins/src/redpanda/vbuild/llvm/install/bin/../include/c++/v1/algorithm:1748:16
    #3 0x558e9c448c9a in std::__1::enable_if<__is_cpp17_random_access_iterator<char const*>::value, char*>::type std::__1::copy_n<char const*, unsigned long, char*>(char const*, unsigned long, char*) /home/nwatkins/src/redpanda/vbuild/llvm/install/bin/../include/c++/v1/algorithm:1
867:12
    #4 0x558e9c674104 in iobuf_copy(details::io_iterator_consumer&, unsigned long)::$_4::operator()(char const*, unsigned long) const /home/nwatkins/src/redpanda/vbuild/debug/clang/../../../src/v/bytes/iobuf.cc:145:13
    #5 0x558e9c65aa69 in unsigned long details::io_iterator_consumer::consume<iobuf_copy(details::io_iterator_consumer&, unsigned long)::$_4>(unsigned long, iobuf_copy(details::io_iterator_consumer&, unsigned long)::$_4&&) /home/nwatkins/src/redpanda/vbuild/debug/clang/../../../sr
c/v/bytes/details/io_iterator_consumer.h:141:45
    #6 0x558e9c659af1 in iobuf_copy(details::io_iterator_consumer&, unsigned long) /home/nwatkins/src/redpanda/vbuild/debug/clang/../../../src/v/bytes/iobuf.cc:143:12
    #7 0x558e9c57eeb3 in iobuf_parser_base::peek(unsigned long) const /home/nwatkins/src/redpanda/vbuild/debug/clang/../../../src/v/bytes/iobuf_parser.h:100:16
    #8 0x558e9c5751b6 in serde::peek_version(iobuf_parser&) /home/nwatkins/src/redpanda/vbuild/debug/clang/../../../src/v/serde/serde.h:547:43
    #9 0x558e9c55ae54 in storage::read_nested(iobuf_parser&, storage::index_state&, unsigned long) /home/nwatkins/src/redpanda/vbuild/debug/clang/../../../src/v/storage/index_state.cc:257:9
    #10 0x558e9c475297 in std::__1::decay<storage::index_state>::type serde::read_nested<storage::index_state>(iobuf_parser&, unsigned long) /home/nwatkins/src/redpanda/vbuild/debug/clang/../../../src/v/serde/serde.h:475:5
    #11 0x558e9c55aedb in storage::read_nested(iobuf_parser&, storage::index_state&, unsigned long) /home/nwatkins/src/redpanda/vbuild/debug/clang/../../../src/v/storage/index_state.cc:258:14
    #12 0x558e9c475297 in std::__1::decay<storage::index_state>::type serde::read_nested<storage::index_state>(iobuf_parser&, unsigned long) /home/nwatkins/src/redpanda/vbuild/debug/clang/../../../src/v/serde/serde.h:475:5
    #13 0x558e9c55aedb in storage::read_nested(iobuf_parser&, storage::index_state&, unsigned long) /home/nwatkins/src/redpanda/vbuild/debug/clang/../../../src/v/storage/index_state.cc:258:14
    #14 0x558e9c475297 in std::__1::decay<storage::index_state>::type serde::read_nested<storage::index_state>(iobuf_parser&, unsigned long) /home/nwatkins/src/redpanda/vbuild/debug/clang/../../../src/v/serde/serde.h:475:5
    #15 0x558e9c55aedb in storage::read_nested(iobuf_parser&, storage::index_state&, unsigned long) /home/nwatkins/src/redpanda/vbuild/debug/clang/../../../src/v/storage/index_state.cc:258:14
    ...

===

This makes the recursion go away, but the deserialized output is all zeros. So that
probably isn't the solution.

diff --git a/src/v/storage/index_state.cc b/src/v/storage/index_state.cc
index 57808eae2..4aceb904f 100644
--- a/src/v/storage/index_state.cc
+++ b/src/v/storage/index_state.cc
@@ -255,7 +255,7 @@ iobuf index_state::checksum_and_serialize() {

 void read_nested(iobuf_parser& in, index_state& st, size_t bytes_left_limit) {
     if (serde::peek_version(in) > index_state::ondisk_version) {
-        st = serde::read_nested<index_state>(in, bytes_left_limit);
+        serde::read_nested<index_state>(in, st, bytes_left_limit);
         return;
     }

Signed-off-by: Noah Watkins <noah@vectorized.io>
  • Loading branch information
dotnwat committed Dec 4, 2021
1 parent f1faa82 commit f2fcb42
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 0 deletions.
24 changes: 24 additions & 0 deletions src/v/storage/index_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,30 @@ struct index_state
static uint64_t checksum_state(const index_state&);
friend std::ostream& operator<<(std::ostream&, const index_state&);

index_state copy() const {
index_state st;
st.size = size;
st.checksum = checksum;
st.bitflags = bitflags;
st.base_offset = base_offset;
st.max_offset = max_offset;
st.base_timestamp = base_timestamp;
st.max_timestamp = max_timestamp;
std::copy(
relative_offset_index.begin(),
relative_offset_index.end(),
std::back_inserter(st.relative_offset_index));
std::copy(
relative_time_index.begin(),
relative_time_index.end(),
std::back_inserter(st.relative_time_index));
std::copy(
position_index.begin(),
position_index.end(),
std::back_inserter(st.position_index));
return st;
}

friend void read_nested(iobuf_parser&, index_state&, size_t);

auto serde_fields() {
Expand Down
12 changes: 12 additions & 0 deletions src/v/storage/tests/index_state_test.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#define BOOST_TEST_MODULE storage
#include "bytes/bytes.h"
#include "random/generators.h"
#include "serde/serde.h"
#include "storage/index_state.h"

#include <boost/test/unit_test.hpp>
Expand Down Expand Up @@ -107,3 +108,14 @@ BOOST_AUTO_TEST_CASE(encode_decode_future_version) {
auto dst = storage::index_state::hydrate_from_buffer(src_buf.copy());
BOOST_REQUIRE(!dst);
}

BOOST_AUTO_TEST_CASE(serde_test) {
auto input = make_random_index_state();
const auto input_copy = input.copy();
BOOST_REQUIRE_EQUAL(input, input_copy);

auto buf = serde::to_iobuf(std::move(input));
const auto output = serde::from_iobuf<storage::index_state>(std::move(buf));

BOOST_REQUIRE_EQUAL(output, input_copy);
}

0 comments on commit f2fcb42

Please sign in to comment.