Implent snapshots support #313

danielealbano · 2023-04-09T23:02:44Z

This PR implements the necessary support to generate the RDB snapshots in the background.

A new fiber has been introduced which takes care of generating a snapshot, depending on the settings. A new set of parameters have been introduced, example below with explanation in line.

  snapshots:
    # The path where the snapshot file will be stored, if the rotation is enabled, the path will be used as prefix and
    # the timestamp of the start of the snapshot will be appended to the file name.
    path: /var/lib/cachegrand/dump.rdb
    # The interval between the snapshots, the allowed units are s, m, h, if not specified the default is seconds.
    interval: 5m
    # The number of keys that must be changed before a snapshot is taken, 0 means no limit
    min_keys_changed: 1000
    # The amount of data that must be changed before a snapshot is taken, the allowed units are b, k, m, g, if not
    min_data_changed: 100mb
    # Rotation settings, optional, if missing the snapshots rotation will be disabled
    rotation:
      # The max number of snapshots files to keep, minimum 2
      max_files: 10

The example is from cachegrand.yaml.skel

The new mechanism takes care of reporting every 3 seconds a status update.

Closes #293

Notes:

Currently the redis commands SAVE and BGSAVE are not implemented, so it's not possible to trigger a backup on demand (Implement the SAVE and BGSAVE Redis commands #314)
The SHUTDOWN command needs to be updated to support SAVE and NOSAVE (The SHUTDOWN command needs to be updated to support the SAVE parameter #315)
The shutdown logic needs to be updated to trigger a dump at the shutdown unless the SHUTDOWN NOSAVE command has been issued (The shutdown logic needs to be updated to trigger a dump at the shutdown unless the SHUTDOWN NOSAVE command has been issued #316)
The current implementation also doesn't compress strings with the LZF algorithm as liblzf is causing segfault and the issue has to be investigated (Investigate liblzf segfaults #312)
Tests for the high level snapshotting process (implemented in storage_db_snapshot.c mostly) are not included in this PR (Tests for the high level snapshotting process (implemented in storage_db_snapshot.c mostly) #317)

… and min_data_changed

…torage db for the snapshots

…ng any action if they are expired

…criptors and implement it both for posix and io_uring

…has been opened

…s writable in the storage_db_snapshot unit, in config check only that the path is not longer than MAX_PATH

… the caller

…ation the snapshot doesn't start

src/storage/db/storage_db_snapshot.h

src/storage/db/storage_db_snapshot.c

src/worker/storage/worker_storage_iouring_op.c

src/worker/storage/worker_storage_posix_op.c

src/storage/db/storage_db_snapshot.c

+        // The LZF compression is disabled for now, it causes a segfault
+//        if (false && likely(!string_serialized || (entry_index->value.size > 32 && entry_index->value.size < 52 * 1024))) {
+//           module_redis_snapshot_serialize_primitive_result_t serialize_result;
+//            size_t allocated_buffer_size = LZF_MAX_COMPRESSED_SIZE(entry_index->value.size) * 1.2;
+//            uint8_t *allocated_buffer = ffma_mem_alloc(allocated_buffer_size);
+//
+//            serialize_result = module_redis_snapshot_serialize_primitive_encode_small_string_lzf(
+//                    string,
+//                    entry_index->value.size,
+//                    allocated_buffer,
+//                    allocated_buffer_size,
+//                    0,
+//                    &buffer_offset);
+//
+//            // If the compression fails or the ration is too low ignore the error, cachegrand will try to
+//            // save the string as a regular string
+//            if (likely(serialize_result == MODULE_REDIS_SNAPSHOT_SERIALIZE_PRIMITIVE_RESULT_OK)) {
+//                if (unlikely(!storage_db_snapshot_rdb_write_buffer(
+//                        db,
+//                        allocated_buffer,
+//                        allocated_buffer_size))) {
+//                    ffma_mem_free(allocated_buffer);
+//                    result = false;
+//                    goto end;
+//                }
+//
+//                string_serialized = true;
+//            }
+//
+//            ffma_mem_free(allocated_buffer);
+//        }


danielealbano · 2023-04-09T23:20:14Z

src/storage/db/storage_db_snapshot.c

+        hashtable_key_size_t key_size = 0;
+
+        // Tries to fetch the next entry within the block being processed
+        bucket_index++;


Required by the implementation

src/worker/storage/worker_storage_iouring_op.c

src/worker/storage/worker_storage_posix_op.c

danielealbano added 30 commits March 24, 2023 20:34

Add a configuration example for the snapshots

cdf7e55

Add snapshots config data structures

15c9ab2

Add schema parsing

8f998fd

Implement a function to parse time strings

b138e5f

Add the snapshots section to the config file schema

c7370ea

Drop function signature declaration from test

d8e6c8f

Add the string to numeric value conversion for the snapshots interval…

b7f8d86

… and min_data_changed

Implement the database snapshots config validation

405399f

Fix data structs for config->database->snapshots

59b736f

Expose config database snapshots validation

5fd07f8

Fix schema for min_keys_changed, no reason to have it as string

cd94467

Update config tests to validate the snapshots section

2a4a128

Merge branch 'main' into feat-impl-snapshots-support

b63ed51

Merge branch 'main' into feat-impl-snapshots-support

8e6d17b

Add the rdb extension to the default path of the snapshots

2e0f378

Merge branch 'main' into feat-impl-snapshots-support

3153acd

Fix type module_redis_snapshot_value_type_t name

1b105ed

Ensure the path is always freed

ef0b9ac

Expose a number of new information in the storage db config and the s…

ddc1b85

…torage db for the snapshots

Fix a bug in how the expired keys were handled

8bdbef7

Expose a new function to prep for read the entry indexes without taki…

7f6ab28

…ng any action if they are expired

Clarify the expiration cleanup logic

41e44a1

Headers cleanup

1cbb4ef

Expose a new internal api call to open storage channels from file des…

dfee360

…criptors and implement it both for posix and io_uring

Initial implementation of the storage_rdb_snapshot

4ef06f9

Initial implementation of the worker fiber to run the snapshots

d1477f1

Set the timestamp for next time to run the snapshot once the storage …

9955af2

…has been opened

Move the logic to ensure that the snapshot parent folder exists and i…

d2e77b3

…s writable in the storage_db_snapshot unit, in config check only that the path is not longer than MAX_PATH

Update tests after refactoring

fda9b8a

Fix the export of the fiber entry point

5f4e072

danielealbano added 11 commits April 10, 2023 00:32

Run the garbage collection more often

2d64c62

Setup the snapshot parameters for the storage db config

9d2d559

Avoid purging too many entries altogether if the list is too long

30e9a2a

Fix storage_db_snapshot_rdb_write_value_header to accept the key from…

9a9bbd4

… the caller

Disable the LZF compression code, it's causing segfaults

322952f

Only strings supported

e38df6e

Ensure that if there are no data changed as requested by the configur…

536ebf3

…ation the snapshot doesn't start

min_keys_changed shouldn't be set to zero

66a4a52

Useless check, the parameter is an unsigned int

94fe2cf

Drop some logging that is not necessary

4a76057

Update a comment in cachegrand.skel.yaml

c61550d

danielealbano added the enhancement New feature or request label Apr 9, 2023

danielealbano added this to the v0.2 milestone Apr 9, 2023

danielealbano added this to In Progress in cachegrand via automation Apr 9, 2023

danielealbano self-assigned this Apr 9, 2023

danielealbano linked an issue Apr 9, 2023 that may be closed by this pull request

Implement redis-compatible (RDB) snapshotting support #293

Closed

danielealbano enabled auto-merge (squash) April 9, 2023 23:02

github-advanced-security bot found potential problems Apr 9, 2023

View reviewed changes

Fix test

f1c8f83

danielealbano disabled auto-merge April 9, 2023 23:12

Fix security warnings reported by codeql

2897401

danielealbano enabled auto-merge (squash) April 9, 2023 23:22

github-advanced-security bot found potential problems Apr 9, 2023

View reviewed changes

src/worker/storage/worker_storage_iouring_op.c Fixed Show fixed Hide fixed

src/worker/storage/worker_storage_posix_op.c Fixed Show fixed Hide fixed

danielealbano added 4 commits April 10, 2023 01:26

Fix the iouring storage backend test

634e3a9

Ensure zero termination

543736c

Fix test

e3ac6cf

Minor style fixes

ad61102

danielealbano merged commit c85fd82 into main Apr 9, 2023

cachegrand automation moved this from In Progress to Completed Apr 9, 2023

danielealbano deleted the feat-impl-snapshots-support branch April 9, 2023 23:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implent snapshots support #313

Implent snapshots support #313

danielealbano commented Apr 9, 2023 •

edited

Loading

danielealbano Apr 9, 2023

Implent snapshots support #313

Implent snapshots support #313

Conversation

danielealbano commented Apr 9, 2023 • edited Loading

danielealbano Apr 9, 2023

Choose a reason for hiding this comment

danielealbano commented Apr 9, 2023 •

edited

Loading