Skip to content

Commit

Permalink
Multi-PRs: Collection of various print / debug code enhancements.
Browse files Browse the repository at this point in the history
(#537) Add unit-tests to exercise rc/allocator.c; Other minor cleanup.

This commit refactors and does some minor cleanup of code in allocator.{h,c}
and couple of related files. Some page / extent number / offset
boolean and conversion macros are added, along with new unit-tests.
No significant change in code-logic is being introduced with this fix.

(#538) Unit-test to exercise mini_allocator. Minor cleanup of module.

This commit does some minor cleanup of the mini-allocator module.
Some interfaces are slightly changed to streamline init / deinit
interfaces. A new unit-test is added to exercise core interfaces
of this page allocation system, and to exercise / test the print
methods for keyed and unkeyed page allocation schemes.

Extend print methods to also print page_addr and extent_addr as
page_num and extent_num, where applicable.

(#530) Add test cases to exercise print methods for mini-allocator metapages

This commit adds new print function to drive the functions for printing
keyed / unkeyed metadata pages of the mini-allocator. Existing test cases
are enhanced as follows to eventually exercise these print methods.

- Add bunch of print functions in trunk.c to find out routing filter
  metadata pages and to invoke underlying print methods on such pages.
  Print pivot key and other filter-related data.

- Extend splinter_test:test_splinter_print_diags() to exercise the print
  function that will walk through routing filter metadata pages for one
  pivot key off of the trunk's root node and eventually callss
  mini_unkeyed_print().

- Add new test_splinter_verbose_print_diags test case to exercise the
  verbose print logging in trunk.c (for a small data set).

- Modularize code in btree_stress_test.c to carve out code that is used
  to build new unit-test case, test_btree_print_diags(). First, this
  invokes btree_print_tree(), to see outputs of packed BTree node.
  Then, invokes mini_keyed_print(), to print keyed mini-allocator's
  metadata pages.

Identify Memtable v/s Branch page types via BTree-print routines.

This commit extends BTree-print routines to also report the page
type, whether it's a branch or a memtable BTree. As the structures and
print methods are shared between two objects, this extra information
will help in diagnostics. Trunk nodes are likewise identified.
Minor fix in trunk_print_pivots() to align outputs for pivot key's string.

Fix compilation errors that show in debug builds. Readjust output formatting.

(#533) Add print fn to print branch BTrees under trunk node.

- Introduce trunk_print_branch_btrees() to walk the root-addresses
  of branch BTree nodes for a given trunk node, and to invoke the
  BTree print methods on each such branch. The top-level BTree
  root node is being printed right, but upon recursing down, we
  are running into an unallocated page error.
  allocator_page_valid() is consistently failing for all sub-trees
  under root branch BTree.

Tentative fix to by-pass "unallocated page errors".

The previous commit runs into issues while trying to print BTree
sub-trees under branch BTree root pages. We run into the following
errors:

btree_print_subtree() -> allocator_page_valid(), which will fail
complaining that "Trying to access an unreferenced extent. ..."
as (refcount==0)

I wondered if this is because we use the mini-allocator to allocate
pages for BTrees, and perhaps, these pages are not being recorded
as referenced in the allocator's reference count.

This commit, thus, implements a by-pass 'fix' to pass-down the page
type. Deep inside allocator_page_valid(), we try to ignore refcount==0
in case the page's type is PAGE_TYPE_BRANCH. This still runs into
the following stack:

---
    linenumber=2108,
    functionname=0x7ffff7f9f9a0 <__FUNCTION__.17> "clockcache_get_internal",
    expr=0x7ffff7f9ee98 "(extent_ref_count > 1)",
    message=0x7ffff7f9edf8 "Attempt to get a buffer for page addr=%lu, page type=%d ('%s'), from extent addr=%lu, (extent number=%lu), which is an unallocated extent, extent_ref_count=%u.") at src/platform_linux/platform.c:370
    blocking=1, type=PAGE_TYPE_BRANCH, page=0x7fffffffe230) at src/clockcache.c:2108
    type=PAGE_TYPE_BRANCH) at src/clockcache.c:2267
    blocking=1, type=PAGE_TYPE_BRANCH) at src/clockcache.c:295
    type=PAGE_TYPE_BRANCH) at src/cache.h:283
    node=0x7fffffffe380, type=PAGE_TYPE_BRANCH) at src/btree.c:1043
    cc=0x555555577040, cfg=0x7fffb550c0c0, node=0x7fffffffe380, type=PAGE_TYPE_BRANCH)
    at src/btree.c:3259
    cc=0x555555577040, cfg=0x7fffb550c0c0, addr=4319870976, type=PAGE_TYPE_BRANCH)
    at src/btree.c:3281
    cc=0x555555577040, cfg=0x7fffb550c0c0, addr=4319608832, type=PAGE_TYPE_BRANCH)
    at src/btree.c:3300
    cc=0x555555577040, cfg=0x7fffb550c0c0, root_addr=4319608832) at src/btree.c:3337
    arg=0x7ffff7ed3780 <_IO_2_1_stdout_>) at src/trunk.c:8943
---

The mystery is how come the same access functions to read BTree sub-tree's
child-pages in btree_lookup_node() seemingly works fine.

One issue is that existing tests that exercise that function for query tests
are always only dealing with PAGE_TYPE_MEMTABLE. This may be the reason why
they don't run into branch BTree pages that appear unallocated.

Needs further consult & investigation.

Fix merge issue.

Print BTree & Trunk addresses as extent & page number. Add unit-tests.

Consistently report page 'addr' as extent number and page number for
BTree and Trunk node page print methods. Add unit test-cases to verify
BTree extent / page-number conversion macros.

(#156) Extend trunk_node_print_branches(). Call it from unit-test print_diags.

This commit adds minor improvements to trunk_node_print_branches() to:

- Print sum of {# of branches, num tuples & num of kv-bytes} for
  each trunk node.
- Add trunk_node_stats{} to gather metrics as we traverse trunk nodes
  during printing. Report one-line summary of aggregate metrics.

- Extend unit-test test_splinter_print_diags() to now also call
  trunk_print_branches() so we have a basic exercise of this
  print method.

Fix merge-issues: Call set_log_streams_for_tests() as needed in new unit-tests.
  • Loading branch information
gapisback committed Feb 8, 2023
1 parent 4b6e0b1 commit cd7a6cf
Show file tree
Hide file tree
Showing 22 changed files with 1,905 additions and 280 deletions.
20 changes: 16 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -392,12 +392,14 @@ PLATFORM_IO_SYS = $(OBJDIR)/$(SRCDIR)/$(PLATFORM_DIR)/laio.o

UTIL_SYS = $(OBJDIR)/$(SRCDIR)/util.o $(PLATFORM_SYS)

CLOCKCACHE_SYS = $(OBJDIR)/$(SRCDIR)/clockcache.o \
$(OBJDIR)/$(SRCDIR)/allocator.o \
$(OBJDIR)/$(SRCDIR)/rc_allocator.o \
ALLOCATOR_SYS = $(OBJDIR)/$(SRCDIR)/allocator.o \
$(OBJDIR)/$(SRCDIR)/rc_allocator.o \
$(PLATFORM_IO_SYS)

CLOCKCACHE_SYS = $(OBJDIR)/$(SRCDIR)/clockcache.o \
$(OBJDIR)/$(SRCDIR)/task.o \
$(ALLOCATOR_SYS) \
$(UTIL_SYS) \
$(PLATFORM_IO_SYS)

BTREE_SYS = $(OBJDIR)/$(SRCDIR)/btree.o \
$(OBJDIR)/$(SRCDIR)/data_internal.o \
Expand Down Expand Up @@ -466,6 +468,16 @@ $(BINDIR)/$(UNITDIR)/platform_apis_test: $(UTIL_SYS) \
$(COMMON_UNIT_TESTOBJ) \
$(PLATFORM_SYS)

$(BINDIR)/$(UNITDIR)/allocator_test: $(ALLOCATOR_SYS) \
$(OBJDIR)/$(TESTS_DIR)/config.o \
$(COMMON_UNIT_TESTOBJ) \
$(UTIL_SYS)

$(BINDIR)/$(UNITDIR)/mini_allocator_test: $(COMMON_TESTOBJ) \
$(OBJDIR)/$(FUNCTIONAL_TESTSDIR)/test_async.o \
$(COMMON_UNIT_TESTOBJ) \
$(LIBDIR)/libsplinterdb.so

########################################
# Convenience mini unit-test targets
unit/util_test: $(BINDIR)/$(UNITDIR)/util_test
Expand Down
29 changes: 29 additions & 0 deletions src/allocator.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,32 @@ allocator_config_init(allocator_config *allocator_cfg,
uint64 log_extent_size = 63 - __builtin_clzll(io_cfg->extent_size);
allocator_cfg->extent_mask = ~((1ULL << log_extent_size) - 1);
}

/*
* Return page number for the page at 'addr', in terms of page-size.
* This routine assume that input 'addr' is a valid page address.
*/
uint64
allocator_page_number(allocator *al, uint64 page_addr)
{
allocator_config *allocator_cfg = allocator_get_config(al);
debug_assert(allocator_valid_page_addr(al, page_addr));
return ((page_addr / allocator_cfg->io_cfg->page_size));
}

/*
* Return page offset for the page at 'addr', in terms of page-size,
* as an index into the extent holding the page.
* This routine assume that input 'addr' is a valid page address.
*
* Returns index from [0 .. ( <#-of-pages-in-an-extent> - 1) ]
*/
uint64
allocator_page_offset(allocator *al, uint64 page_addr)
{
allocator_config *allocator_cfg = allocator_get_config(al);
debug_assert(allocator_valid_page_addr(al, page_addr));
uint64 npages_in_extent =
(allocator_cfg->io_cfg->extent_size / allocator_cfg->io_cfg->page_size);
return (allocator_page_number(al, page_addr) % npages_in_extent);
}
71 changes: 64 additions & 7 deletions src/allocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ allocator_config_init(allocator_config *allocator_cfg,
io_config *io_cfg,
uint64 capacity);

// Return the address of the extent holding page at address 'addr'
static inline uint64
allocator_config_extent_base_addr(allocator_config *allocator_cfg, uint64 addr)
{
Expand Down Expand Up @@ -253,12 +254,50 @@ allocator_print_allocated(allocator *al)
return al->ops->print_allocated(al);
}

// Return the address of the extent holding page at address 'addr'
static inline uint64
allocator_extent_base_addr(allocator *al, uint64 addr)
{
allocator_config *allocator_cfg = allocator_get_config(al);
return allocator_config_extent_base_addr(allocator_cfg, addr);
}

// Returns the address of the page next to input 'page_addr'
static inline uint64
allocator_next_page_addr(allocator *al, uint64 page_addr)
{
allocator_config *allocator_cfg = allocator_get_config(al);
return (page_addr + allocator_cfg->io_cfg->page_size);
}


static inline bool
allocator_page_valid(allocator *al, uint64 addr)
allocator_valid_page_addr(allocator *al, uint64 addr)
{
allocator_config *allocator_cfg = allocator_get_config(al);
return ((addr % allocator_cfg->io_cfg->page_size) == 0);
}

if ((addr % allocator_cfg->io_cfg->page_size) != 0) {
/*
* Is the 'addr' a valid address of the start of an extent;
* i.e. an extent address?
*/
static inline bool
allocator_valid_extent_addr(allocator *al, uint64 addr)
{
return (allocator_extent_base_addr(al, addr) == addr);
}

/*
* Check if the page given by address 'addr' is a valid page-address within the
* database capacity and that the holding extent is also allocated (i.e., has a
* non-zero ref-count).
*/
static inline bool
allocator_page_valid(allocator *al, uint64 addr, page_type type)
{
allocator_config *allocator_cfg = allocator_get_config(al);
if (!allocator_valid_page_addr(al, addr)) {
platform_error_log("%s():%d: Specified addr=%lu is not divisible by"
" configured page size=%lu\n",
__FUNCTION__,
Expand All @@ -268,10 +307,10 @@ allocator_page_valid(allocator *al, uint64 addr)
return FALSE;
}

uint64 base_addr = allocator_config_extent_base_addr(allocator_cfg, addr);
if ((base_addr != 0) && (addr < allocator_cfg->capacity)) {
uint64 base_addr = allocator_extent_base_addr(al, addr);
if ((base_addr != 0) && (addr < allocator_get_capacity(al))) {
uint8 refcount = allocator_get_refcount(al, base_addr);
if (refcount == 0) {
if ((refcount == 0) && (type != PAGE_TYPE_BRANCH)) {
platform_error_log(
"%s():%d: Trying to access an unreferenced extent."
" base_addr=%lu, addr=%lu, allocator_get_refcount()=%d\n",
Expand All @@ -281,11 +320,11 @@ allocator_page_valid(allocator *al, uint64 addr)
addr,
refcount);
}
return (refcount != 0);
return ((refcount != 0) || (type == PAGE_TYPE_BRANCH));
} else {
platform_error_log("%s():%d: Extent out of allocator capacity range."
" base_addr=%lu, addr=%lu"
", allocator_get_capacity()=%lu\n",
", allocator_get_capacity()=%lu pages.\n",
__FUNCTION__,
__LINE__,
base_addr,
Expand All @@ -294,3 +333,21 @@ allocator_page_valid(allocator *al, uint64 addr)
return FALSE;
}
}

/*
* Return extent number of the extent holding the page at 'addr'.
* This routine assume that input 'addr' is a valid page address.
*/
static inline uint64
allocator_extent_number(allocator *al, uint64 page_addr)
{
allocator_config *allocator_cfg = allocator_get_config(al);
debug_assert(allocator_valid_page_addr(al, page_addr));
return ((allocator_extent_base_addr(al, page_addr)
/ allocator_cfg->io_cfg->extent_size));
}
uint64
allocator_page_number(allocator *al, uint64 addr);

uint64
allocator_page_offset(allocator *al, uint64 page_addr);
Loading

0 comments on commit cd7a6cf

Please sign in to comment.