Fix osd_peering_pg_test

- Fix broken garbage collection in the new store leading to very high memory usage
- Make the new store do full garbage collection on OSD startup by default (can be turned off with a new gc_on_start parameter)
- Make startup logging with the new store more informative, allow to log amount of garbage with log_level > 1
- Fix clients reading from snapshots incorrectly on retries (for example when OSDs were restarted during read)
- Fix a possible "send-after-free" in OSD which could probably also affect snapshot reads
- Fix OSD crashing when handling a read_bitmap operation from a deleted object with the new store

CMakeLists.txt

@@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 2.8...3.30)
 
 project(vitastor)
 
-set(VITASTOR_VERSION "3.0.8")
+set(VITASTOR_VERSION "3.0.9")
 
 include(CTest)
 

csi/Makefile

@@ -1,4 +1,4 @@
-VITASTOR_VERSION ?= v3.0.8
+VITASTOR_VERSION ?= v3.0.9
 
 all: build push
 

csi/deploy/004-csi-nodeplugin.yaml

@@ -49,7 +49,7 @@ spec:
             capabilities:
               add: ["SYS_ADMIN"]
             allowPrivilegeEscalation: true
-          image: vitalif/vitastor-csi:v3.0.8
+          image: vitalif/vitastor-csi:v3.0.9
           args:
             - "--node=$(NODE_ID)"
             - "--endpoint=$(CSI_ENDPOINT)"

csi/deploy/007-csi-provisioner.yaml

@@ -121,7 +121,7 @@ spec:
             privileged: true
             capabilities:
               add: ["SYS_ADMIN"]
-          image: vitalif/vitastor-csi:v3.0.8
+          image: vitalif/vitastor-csi:v3.0.9
           args:
             - "--node=$(NODE_ID)"
             - "--endpoint=$(CSI_ENDPOINT)"

csi/src/config.go

@@ -5,7 +5,7 @@ package vitastor
 
 const (
     vitastorCSIDriverName    = "csi.vitastor.io"
-    vitastorCSIDriverVersion = "3.0.8"
+    vitastorCSIDriverVersion = "3.0.9"
 )
 
 // Config struct fills the parameters of request or user input

debian/changelog

@@ -1,4 +1,4 @@
-vitastor (3.0.8-1) unstable; urgency=medium
+vitastor (3.0.9-1) unstable; urgency=medium
 
   * Bugfixes
 

docker/Makefile

@@ -1,4 +1,4 @@
-VITASTOR_VERSION ?= v3.0.8
+VITASTOR_VERSION ?= v3.0.9
 
 all: build push
 

docker/etc/vitastor/docker.conf

@@ -4,7 +4,7 @@
 #
 
 # Desired Vitastor version
-VITASTOR_VERSION=v3.0.8
+VITASTOR_VERSION=v3.0.9
 
 # Additional arguments for all containers
 # For example, you may want to specify a custom logging driver here

docs/config/osd.en.md

@@ -70,6 +70,7 @@ with an OSD restart or, for some of them, even without restarting by updating co
 - [use_atomic_flag](#use_atomic_flag)
 - [pg_reshard_chunk_size](#pg_reshard_chunk_size)
 - [pg_reshard_chunk_pause_ms](#pg_reshard_chunk_pause_ms)
+- [gc_on_start](#gc_on_start)
 
 ## bind_address
 
@@ -753,3 +754,9 @@ This option sets the maximum number of object is a chunk. Moving 100k objects us
 - Default: 100
 
 This option sets the interval between handling two PG count change chunks.
+
+## gc_on_start
+
+- Type: boolean
+
+Forcibly clean all garbage entries in the new store on every OSD restart.

docs/config/osd.ru.md

@@ -71,6 +71,7 @@
 - [use_atomic_flag](#use_atomic_flag)
 - [pg_reshard_chunk_size](#pg_reshard_chunk_size)
 - [pg_reshard_chunk_pause_ms](#pg_reshard_chunk_pause_ms)
+- [gc_on_start](#gc_on_start)
 
 ## bind_address
 
@@ -793,3 +794,9 @@ pg_minsize OSD во время переключений, что может по
 - Значение по умолчанию: 100
 
 Данная опция задаёт интервал между обработкой двух порций изменения числа PG пулов.
+
+## gc_on_start
+
+- Тип: булево (да/нет)
+
+Принудительно очищать все мусорные записи в новом хранилище при каждом запуске OSD.

docs/config/src/osd.yml

@@ -938,3 +938,7 @@
     This option sets the interval between handling two PG count change chunks.
   info_ru: |
     Данная опция задаёт интервал между обработкой двух порций изменения числа PG пулов.
+- name: gc_on_start
+  type: bool
+  info: Forcibly clean all garbage entries in the new store on every OSD restart.
+  info_ru: Принудительно очищать все мусорные записи в новом хранилище при каждом запуске OSD.

docs/installation/docker.en.md

@@ -26,9 +26,9 @@ at Vitastor Kubernetes operator: https://github.com/Antilles7227/vitastor-operat
 The instruction is very simple.
 
 1. Download a Docker image of the desired version: \
-   `docker pull vitalif/vitastor:v3.0.8`
+   `docker pull vitalif/vitastor:v3.0.9`
 2. Install scripts to the host system: \
-   `docker run --rm -it -v /etc:/host-etc -v /usr/bin:/host-bin vitalif/vitastor:v3.0.8 install.sh`
+   `docker run --rm -it -v /etc:/host-etc -v /usr/bin:/host-bin vitalif/vitastor:v3.0.9 install.sh`
 3. Reload udev rules: \
    `udevadm control --reload-rules`
 4. Enable the vitastor-host service: \

docs/installation/docker.ru.md

@@ -25,9 +25,9 @@ Vitastor можно установить в Docker/Podman. При этом etcd,
 Инструкция по установке максимально простая.
 
 1. Скачайте Docker-образ желаемой версии: \
-   `docker pull vitalif/vitastor:v3.0.8`
+   `docker pull vitalif/vitastor:v3.0.9`
 2. Установите скрипты в хост-систему командой: \
-   `docker run --rm -it -v /etc:/host-etc -v /usr/bin:/host-bin vitalif/vitastor:v3.0.8 install.sh`
+   `docker run --rm -it -v /etc:/host-etc -v /usr/bin:/host-bin vitalif/vitastor:v3.0.9 install.sh`
 3. Перезагрузите правила udev: \
    `udevadm control --reload-rules`
 4. Включите сервис vitastor-host: \

mon/package.json

@@ -1,6 +1,6 @@
 {
   "name": "vitastor-mon",
-  "version": "3.0.8",
+  "version": "3.0.9",
   "description": "Vitastor SDS monitor service",
   "main": "mon-main.js",
   "scripts": {

mon/stats.js

@@ -37,6 +37,7 @@ function derive_osd_stats(st, prev, prev_diff)
         const n = c.count - BigInt(pr && pr.count||0);
         diff.recovery_stats[op] = { ...c, bps: n > 0 ? b*1000n/timediff : 0n, iops: n > 0 ? n*1000n/timediff : 0n };
     }
+    diff.inode_stats = {};
     for (const pool_id in st.inode_stats||{})
     {
         diff.inode_stats[pool_id] = {};

node-binding/package.json

@@ -1,6 +1,6 @@
 {
   "name": "vitastor",
-  "version": "3.0.8",
+  "version": "3.0.9",
   "description": "Low-level native bindings to Vitastor client library",
   "main": "index.js",
   "keywords": [

patches/cinder-vitastor.py

@@ -50,7 +50,7 @@ from cinder.volume import configuration
 from cinder.volume import driver
 from cinder.volume import volume_utils
 
-VITASTOR_VERSION = '3.0.8'
+VITASTOR_VERSION = '3.0.9'
 
 LOG = logging.getLogger(__name__)
 

rpm/vitastor-el10.spec

@@ -1,11 +1,11 @@
 Name:           vitastor
-Version:        3.0.8
+Version:        3.0.9
 Release:        1%{?dist}
 Summary:        Vitastor, a fast software-defined clustered block storage
 
 License:        Vitastor Network Public License 1.1
 URL:            https://vitastor.io/
-Source0:        vitastor-3.0.8.el10.tar.gz
+Source0:        vitastor-3.0.9.el10.tar.gz
 
 BuildRequires:  gperftools-devel
 BuildRequires:  gcc-c++

rpm/vitastor-el7.spec

@@ -1,11 +1,11 @@
 Name:           vitastor
-Version:        3.0.8
+Version:        3.0.9
 Release:        1%{?dist}
 Summary:        Vitastor, a fast software-defined clustered block storage
 
 License:        Vitastor Network Public License 1.1
 URL:            https://vitastor.io/
-Source0:        vitastor-3.0.8.el7.tar.gz
+Source0:        vitastor-3.0.9.el7.tar.gz
 
 BuildRequires:  gperftools-devel
 BuildRequires:  devtoolset-9-gcc-c++

rpm/vitastor-el8.spec

@@ -1,11 +1,11 @@
 Name:           vitastor
-Version:        3.0.8
+Version:        3.0.9
 Release:        1%{?dist}
 Summary:        Vitastor, a fast software-defined clustered block storage
 
 License:        Vitastor Network Public License 1.1
 URL:            https://vitastor.io/
-Source0:        vitastor-3.0.8.el8.tar.gz
+Source0:        vitastor-3.0.9.el8.tar.gz
 
 BuildRequires:  gperftools-devel
 BuildRequires:  gcc-toolset-9-gcc-c++

rpm/vitastor-el9.spec

@@ -1,11 +1,11 @@
 Name:           vitastor
-Version:        3.0.8
+Version:        3.0.9
 Release:        1%{?dist}
 Summary:        Vitastor, a fast software-defined clustered block storage
 
 License:        Vitastor Network Public License 1.1
 URL:            https://vitastor.io/
-Source0:        vitastor-3.0.8.el9.tar.gz
+Source0:        vitastor-3.0.9.el9.tar.gz
 
 BuildRequires:  gperftools-devel
 BuildRequires:  gcc-c++

src/CMakeLists.txt

@@ -20,7 +20,7 @@ if("${CMAKE_INSTALL_PREFIX}" MATCHES "^/usr/local/?$")
 endif()
 set(ENABLE_COVERAGE false CACHE BOOL "Enable code coverage")
 
-add_definitions(-DVITASTOR_VERSION="3.0.8")
+add_definitions(-DVITASTOR_VERSION="3.0.9")
 add_definitions(-D_GNU_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Wall -Wno-sign-compare -Wno-comment -Wno-parentheses -Wno-pointer-arith -fdiagnostics-color=always -fno-omit-frame-pointer -fvisibility=hidden -I ${CMAKE_SOURCE_DIR}/src)
 add_link_options(-fno-omit-frame-pointer)
 if (${WITH_ASAN})

src/blockstore/blockstore_heap.cpp

@@ -334,19 +334,12 @@ corrupted_block:
                     block_num, block_offset, wr->size, sizeof(heap_entry_t));
                 goto corrupted_block;
             }
-            if (wr->is_garbage())
-            {
-                // Garbage collection is only performed when writing new entries into the block
-                // because it needs a fake LSN and modified blocks require consecutive modified LSNs
-                // That's why garbage entries may persist on disk
-                if (log_level > 5)
-                {
-                    fprintf(stderr, "Notice: skipping garbage entry %jx:%jx v%ju l%ju in metadata block %u at %u\n",
-                        wr->inode, wr->stripe, wr->version, wr->lsn, block_num, block_offset);
-                }
-                block_offset += wr->size;
-                continue;
-            }
+            // Garbage collection is only performed when writing new entries into the block
+            // because it needs a fake LSN and modified blocks require consecutive modified LSNs
+            // At the same time, further modifications _after_ putting new entries into the block,
+            // but _before_ writing it, may mark some entries in it as garbage. That's why garbage
+            // entries may still be present on disk.
+            wr->entry_type &= ~BS_HEAP_GARBAGE;
             if ((wr->entry_type & BS_HEAP_TYPE) < BS_HEAP_BIG_WRITE ||
                 (wr->entry_type & BS_HEAP_TYPE) > BS_HEAP_ROLLBACK ||
                 (wr->entry_type & ~(BS_HEAP_TYPE|BS_HEAP_STABLE)) ||

src/blockstore/blockstore_init.cpp

@@ -311,7 +311,7 @@ resume_8:
         uint32_t block_num = recheck_mod[i];
         uint64_t block_offset = bs->dsk.meta_offset + (uint64_t)(block_num+1) * bs->dsk.meta_block_size;
         data = ((ring_data_t*)sqe->user_data);
-        uint8_t *buf = (uint8_t*)malloc_or_die(bs->dsk.meta_block_size);
+        uint8_t *buf = (uint8_t*)memalign_or_die(MEM_ALIGNMENT, bs->dsk.meta_block_size);
         bs->heap->get_meta_block(block_num, buf);
         data->iov = { buf, bs->dsk.meta_block_size };
         data->callback = [this, buf, block_offset](ring_data_t *data)

src/blockstore/v1/impl.cpp

@@ -877,7 +877,7 @@ uint64_t blockstore_impl_t::get_garbage_entries()
 
 uint64_t blockstore_impl_t::get_garbage_memory()
 {
-    return sizeof(blockstore_dirty_db_t::node_type) * dirty_db.size();
+    return (sizeof(obj_ver_id) + sizeof(dirty_entry) + 32) * dirty_db.size();
 }
 
 } // namespace v1

src/client/cluster_client.cpp

@@ -590,7 +590,7 @@ void cluster_client_t::on_change_pool_config_hook()
         {
             if (log_level > 2 && pg_counts[pool_item.first])
             {
-                printf("Pool %u (%s) PG count changed from %lu to %lu\n", pool_item.first, pool_item.second.name.c_str(),
+                fprintf(stderr, "Pool %u (%s) PG count changed from %lu to %lu\n", pool_item.first, pool_item.second.name.c_str(),
                     pg_counts[pool_item.first], pool_item.second.real_pg_count);
             }
             // At this point, all pool operations should have been suspended

src/client/msgr_rdma.cpp

@@ -187,6 +187,8 @@ std::vector<msgr_rdma_context_t*> msgr_rdma_context_t::create_all(const std::vec
     ibv_device **raw_dev_list = NULL;
     ibv_device **dev_list = NULL;
     ibv_device *single_list[2] = {};
+    int up_ports = 0;
+    int single_port_num = 0;
 
     raw_dev_list = dev_list = ibv_get_device_list(NULL);
     if (!dev_list || !*dev_list)
@@ -221,6 +223,7 @@ std::vector<msgr_rdma_context_t*> msgr_rdma_context_t::create_all(const std::vec
         dev_list = single_list;
     }
 
+retry:
     for (int i = 0; dev_list[i]; ++i)
     {
         auto dev = dev_list[i];
@@ -258,6 +261,9 @@ std::vector<msgr_rdma_context_t*> msgr_rdma_context_t::create_all(const std::vec
                 fprintf(stderr, "RDMA device %s port %d GID %d does not exist\n", ibv_get_device_name(dev), port_num, sel_gid_index);
                 continue;
             }
+            up_ports++;
+            single_port_num = port_num;
+            single_list[0] = dev;
             uint32_t port_mtu = sel_mtu ? sel_mtu : ibv_mtu_to_bytes(portinfo.active_mtu);
 #ifdef IBV_ADVISE_MR_ADVICE_PREFETCH_NO_FAULT
             if (sel_gid_index < 0)
@@ -298,6 +304,14 @@ cleanup_dev:
         ibv_close_device(context);
     }
 
+    if (!ret.size() && up_ports == 1 && dev_list != single_list)
+    {
+        // Auto-select the only available device/port if there is only one
+        dev_list = single_list;
+        sel_port_num = single_port_num;
+        goto retry;
+    }
+
 cleanup:
     if (raw_dev_list)
         ibv_free_device_list(raw_dev_list);

src/client/vitastor.pc.in

@@ -6,7 +6,7 @@ includedir=${prefix}/@CMAKE_INSTALL_INCLUDEDIR@
 
 Name: Vitastor
 Description: Vitastor client library
-Version: 3.0.8
+Version: 3.0.9
 Libs: -L${libdir} -lvitastor_client
 Cflags: -I${includedir}
 

src/osd/osd_peering_pg_test.cpp

@@ -1,8 +1,6 @@
 // Copyright (c) Vitaliy Filippov, 2019+
 // License: VNPL-1.1 (see README.md for details)
 
-#define _LARGEFILE64_SOURCE
-
 #include "malloc_or_die.h"
 #include "osd_peering_pg.h"
 #define STRIPE_SHIFT 12
@@ -22,13 +20,20 @@
  */
 int main(int argc, char *argv[])
 {
+    setvbuf(stdout, NULL, _IONBF, 0);
+    setvbuf(stderr, NULL, _IONBF, 0);
     pg_t pg = {
         .state = PG_PEERING,
+        .scheme = POOL_SCHEME_EC,
+        .pg_size = 3,
+        .pg_minsize = 2,
+        .pg_data_size = 2,
         .pg_num = 1,
         .target_set = { 1, 2, 3 },
         .cur_set = { 1, 2, 3 },
         .peering_state = new pg_peering_state_t(),
     };
+    printf("generating\n");
     for (uint64_t osd_num = 1; osd_num <= 3; osd_num++)
     {
         pg_list_result_t r = {
@@ -48,8 +53,15 @@ int main(int argc, char *argv[])
         }
         pg.peering_state->list_results[osd_num] = r;
     }
+    printf("calculating states\n");
+    timespec tv0;
+    clock_gettime(CLOCK_REALTIME, &tv0);
     pg.calc_object_states(0);
-    printf("deviation variants=%jd clean=%ju\n", pg.state_dict.size(), pg.clean_count);
+    timespec tv1;
+    clock_gettime(CLOCK_REALTIME, &tv1);
+    printf("%.2f deviation variants=%jd clean=%ju\n",
+        (tv1.tv_sec - tv0.tv_sec + tv1.tv_nsec/1000000000.0 - tv0.tv_nsec/1000000000.0),
+        pg.state_dict.size(), pg.clean_count);
     for (auto it: pg.state_dict)
     {
         printf("dev: state=%jx\n", it.second.state);

src/test/test_heap.cpp

@@ -1893,7 +1893,8 @@ void test_big_intent_csums()
         heap.finish_recheck();
 
         auto mod = heap.get_recheck_modified_blocks();
-        assert(mod.size() == 0);
+        assert(mod.size() == 1);
+        assert(mod[0] == 0);
 
         // read object 1 - big_intent should be there
         object_id oid = { .inode = INODE_WITH_POOL(1, 1), .stripe = 0 };