(vmware#561) Fix bug in routing_filter_prefetch(), causing assertion …

…to trip.

This commit fixes a simple arithmetic error in routing_filter_prefetch()
while computing next page's address. The bug results in a debug-assert
in clockcache_get_internal(), or an unending hang in clockcache_get()
code-flow using release binary.

A new test case test_issue_458_mini_destroy_unused_debug_assert has been
added which reproduces the problem. However, this case still runs into
another failure (being tracked separately), so this case is currently
being skipped.

src/btree.c

@@ -2986,7 +2986,11 @@ btree_pack(btree_pack_req *req)
    while (SUCCESS(iterator_at_end(req->itor, &at_end)) && !at_end) {
       iterator_get_curr(req->itor, &tuple_key, &data);
       if (!btree_pack_can_fit_tuple(req, tuple_key, data)) {
-         platform_error_log("btree_pack exceeded output size limit\n");
+         platform_error_log("%s(): req->num_tuples=%lu exceeded output size "
+                            "limit, req->max_tuples=%lu\n",
+                            __func__,
+                            req->num_tuples,
+                            req->max_tuples);
          btree_pack_abort(req);
          return STATUS_LIMIT_EXCEEDED;
       }

src/clockcache.c

@@ -2088,7 +2088,9 @@ clockcache_get_internal(clockcache   *cc,       // IN
                         page_type     type,     // IN
                         page_handle **page)     // OUT
 {
-   debug_assert(addr % clockcache_page_size(cc) == 0);
+   debug_only uint64 page_size = clockcache_page_size(cc);
+   debug_assert(
+      ((addr % page_size) == 0), "addr=%lu, page_size=%lu\n", addr, page_size);
    uint32            entry_number = CC_UNMAPPED_ENTRY;
    uint64            lookup_no    = clockcache_divide_by_page_size(cc, addr);
    debug_only uint64 base_addr =

src/routing_filter.c

@@ -634,14 +634,14 @@ routing_filter_prefetch(cache          *cc,
                         uint64          num_indices)
 {
    uint64 last_extent_addr = 0;
-   uint64 addrs_per_page =
-      cache_config_page_size(cfg->cache_cfg) / sizeof(uint64);
-   uint64 num_index_pages = (num_indices - 1) / addrs_per_page + 1;
-   uint64 index_no        = 0;
+   uint64 page_size        = cache_config_page_size(cfg->cache_cfg);
+   uint64 addrs_per_page   = page_size / sizeof(uint64);
+   uint64 num_index_pages  = (num_indices - 1) / addrs_per_page + 1;
+   uint64 index_no         = 0;
 
    for (uint64 index_page_no = 0; index_page_no < num_index_pages;
         index_page_no++) {
-      uint64       index_addr = filter->addr + index_page_no;
+      uint64       index_addr = filter->addr + (page_size * index_page_no);
       page_handle *index_page =
          cache_get(cc, index_addr, TRUE, PAGE_TYPE_FILTER);
       platform_assert(index_no < num_indices);

tests/unit/splinterdb_stress_test.c

@@ -52,7 +52,8 @@ CTEST_SETUP(splinterdb_stress)
                                              .cache_size = 1000 * Mega,
                                              .disk_size  = 9000 * Mega,
                                              .data_cfg   = &data->default_data_config,
-                                             .queue_scale_percent = 100};
+                                             .num_memtable_bg_threads = 2,
+                                             .num_normal_bg_threads   = 2};
    size_t max_key_size = TEST_KEY_SIZE;
    default_data_config_init(max_key_size, data->cfg.data_cfg);
 
@@ -138,6 +139,59 @@ CTEST2(splinterdb_stress, test_naive_range_delete)
    }
 }
 
+/*
+ * Test case that inserts large # of KV-pairs, and goes into a code path
+ * reported by issue# 458, tripping a debug assert. This test case also
+ * triggered the failure(s) reported by issue # 545.
+ * FIXME: This test still runs into an assertion "filter->addr != 0"
+ * from trunk_inc_filter(), which is being triaged separately.
+ */
+CTEST2_SKIP(splinterdb_stress, test_issue_458_mini_destroy_unused_debug_assert)
+{
+   char key_data[TEST_KEY_SIZE];
+   char val_data[TEST_VALUE_SIZE];
+
+   uint64 test_start_time = platform_get_timestamp();
+
+   for (uint64 ictr = 0, jctr = 0; ictr < 100; ictr++) {
+
+      uint64 start_time = platform_get_timestamp();
+
+      for (jctr = 0; jctr < MILLION; jctr++) {
+
+         uint64 id = (ictr * MILLION) + jctr;
+         snprintf(key_data, sizeof(key_data), "%lu", id);
+         snprintf(val_data, sizeof(val_data), "Row-%lu", id);
+
+         slice key = slice_create(strlen(key_data), key_data);
+         slice val = slice_create(strlen(val_data), val_data);
+
+         int rc = splinterdb_insert(data->kvsb, key, val);
+         ASSERT_EQUAL(0, rc);
+      }
+      uint64 elapsed_ns      = platform_timestamp_elapsed(start_time);
+      uint64 test_elapsed_ns = platform_timestamp_elapsed(test_start_time);
+
+      uint64 elapsed_s = NSEC_TO_SEC(elapsed_ns);
+      if (elapsed_s == 0) {
+         elapsed_s = 1;
+      }
+      uint64 test_elapsed_s = NSEC_TO_SEC(test_elapsed_ns);
+      if (test_elapsed_s == 0) {
+         test_elapsed_s = 1;
+      }
+
+      CTEST_LOG_INFO(
+         "\n" // PLATFORM_CR
+         "Inserted %lu million KV-pairs"
+         ", this batch: %lu s, %lu rows/s, cumulative: %lu s, %lu rows/s ...",
+         (ictr + 1),
+         elapsed_s,
+         (jctr / elapsed_s),
+         test_elapsed_s,
+         (((ictr + 1) * jctr) / test_elapsed_s));
+   }
+}
 
 // Per-thread workload
 static void *