Shrink writecache #1298

dstepanov-yadro · 2024-08-06T13:52:39Z

dstepanov-yadro commented

2024-08-06 13:52:39 +00:00

frostfs-cli control writecache seal supports restore-mode flag to restore writecache's mode to current mode after sealing (writecache seal assumes flush and move to disabled mode)
frostfs-cli control writecache seal supports shrink flag to recreate db after flush to reduce disk size used by bbolt database

* `frostfs-cli control writecache seal` supports `restore-mode` flag to restore writecache's mode to current mode after sealing (`writecache seal` assumes flush and move to disabled mode) * `frostfs-cli control writecache seal` supports `shrink` flag to recreate db after flush to reduce disk size used by bbolt database

dstepanov-yadro force-pushed feat/drop_writecache_bolt_db from cee87c13ca to ba50e46c30

2024-08-06 13:56:05 +00:00

Compare

dstepanov-yadro requested review from storage-core-committers 2024-08-06 14:41:12 +00:00

dstepanov-yadro requested review from storage-core-developers 2024-08-06 14:41:12 +00:00

dstepanov-yadro force-pushed feat/drop_writecache_bolt_db from ba50e46c30 to ef2cc1df3b

2024-08-07 08:20:36 +00:00

Compare

acid-ant approved these changes 2024-08-07 10:16:17 +00:00

achuprov approved these changes 2024-08-07 14:28:53 +00:00

aarifullin approved these changes 2024-08-08 08:30:28 +00:00

fyrchik reviewed 2024-08-08 09:50:02 +00:00

cmd/frostfs-cli/modules/control/writecache.go

					
				@ -68,6 +77,8 @@ func initControlShardsWritecacheCmd() {

					ff.StringSlice(shardIDFlag, nil, "List of shard IDs in base58 encoding")

					ff.Bool(shardAllFlag, false, "Process all shards")

					ff.Bool(ignoreErrorsFlag, true, "Skip invalid/unreadable objects")

					ff.Bool(restoreModeFlag, false, "Restore writecache's mode after sealing")

fyrchik commented

2024-08-08 09:50:02 +00:00

We replaced flush-cache with seal specifically to denote it's meaning in the name.
With this flag it is no longer seal.
Why is it neccessary to have this flag?

We replaced `flush-cache` with `seal` specifically to denote it's meaning in the name. With this flag it is no longer `seal`. Why is it neccessary to have this flag?

dstepanov-yadro commented

2024-08-08 10:46:50 +00:00

It was the real case, when we needed to flush and recreate writecache's bbolt DB.
Agree, with this flag it looks like step back. But writecache seal will have async execution, so I decided to add this flag here.

It was the real case, when we needed to flush and recreate writecache's bbolt DB. Agree, with this flag it looks like step back. But `writecache seal` will have async execution, so I decided to add this flag here.

fyrchik commented

2024-08-08 11:30:31 +00:00

OK, I am too tired to argue.

The help message needs to be altered somehow

$ bin/frostfs-cli control shards writecache seal -h
Flush all the objects from the write-cache to the main storage and move the write-cache to the degraded read only mode: write-cache will be empty and no objects will be put in it.

OK, I am too tired to argue. The help message needs to be altered somehow ``` $ bin/frostfs-cli control shards writecache seal -h Flush all the objects from the write-cache to the main storage and move the write-cache to the degraded read only mode: write-cache will be empty and no objects will be put in it. ```

dstepanov-yadro commented

2024-08-08 12:53:57 +00:00

No need to argue. You asked a question, I answered it. If you do not agree with this decision, then tell me how to do it.

fyrchik marked this conversation as resolved

fyrchik approved these changes 2024-08-08 11:30:59 +00:00

pkg/local_object_storage/writecache/mode.go Outdated

					
				@ -26,3 +29,3 @@

					defer c.modeMtx.Unlock()

					err := c.setMode(ctx, m, true)

					err := c.setMode(ctx, m, true, false)

fyrchik commented

2024-08-08 11:25:26 +00:00

What about prm struct for this? true, false is indistinguishable from false, true
Or maybe bitmask flags.

What about `prm` struct for this? `true, false` is indistinguishable from `false, true` Or maybe bitmask flags.

dstepanov-yadro commented

2024-08-08 13:34:09 +00:00

Fixed

fyrchik marked this conversation as resolved

pkg/local_object_storage/writecache/mode.go Outdated

					
				@ -74,0 +88,4 @@

						b := tx.Bucket(defaultBucket)

						cs := b.Cursor()

						k, v := cs.First()

						empty = len(k) == 0 && len(v) == 0

fyrchik commented

2024-08-08 11:16:15 +00:00

b.Stats().KeyN?

`b.Stats().KeyN`?

dstepanov-yadro commented

2024-08-08 13:34:23 +00:00

Fixed

fyrchik marked this conversation as resolved

pkg/local_object_storage/writecache/mode.go Outdated

					
				@ -74,0 +100,4 @@

					if empty {

						err := os.Remove(filepath.Join(c.path, dbName))

						if err != nil && !os.IsNotExist(err) {

							return fmt.Errorf("failed to remove DB file: %w", err)

fyrchik commented

2024-08-08 11:20:10 +00:00

The function seems no longer idempotent because of this: if we retry, we will fail on c.db.View with ErrClosed

The function seems no longer idempotent because of this: if we retry, we will fail on `c.db.View` with `ErrClosed`

dstepanov-yadro commented

2024-08-08 13:35:04 +00:00

Good point. Fixed. Now assume already closed DB as non empty.

fyrchik marked this conversation as resolved

pkg/services/control/service.proto

					
				@ -657,1 +657,4 @@

				    bool ignore_errors = 2;

				    // If true, then writecache will be sealed, but mode will be restored to the current one.

				    bool restore_mode = 4;