Switch container size metric from physical to logical capacity #42

2023-02-02T20:29:13Z

ir0nbee commented

2023-02-02 20:29:13 +00:00

(Migrated from github.com)

Closes #47

carpawell (Migrated from github.com) reviewed 2023-02-02 20:29:13 +00:00

fyrchik (Migrated from github.com) reviewed 2023-02-02 20:29:13 +00:00

alexvanin (Migrated from github.com) reviewed 2023-02-08 18:14:48 +00:00

cmd/frostfs-node/object.go

alexvanin (Migrated from github.com) commented

2023-02-08 18:13:39 +00:00

Looks like debug change.

acid-ant (Migrated from github.com) reviewed 2023-02-09 09:29:23 +00:00

fyrchik (Migrated from github.com) reviewed 2023-02-10 11:03:42 +00:00

CHANGELOG.md

					
				@ -11,6 +11,7 @@ Changelog for FrostFS Node

				- Add command `frostfs-adm morph netmap-candidates` (#1889)

				### Changed

				- Change `frostfs_node_engine_container_size` to counting sizes of logical objects

fyrchik (Migrated from github.com) commented

2023-02-10 10:52:59 +00:00

to count?

`to count`?

pkg/local_object_storage/metabase/inhume.go

fyrchik (Migrated from github.com) commented

2023-02-10 11:03:30 +00:00

We use and append them together, why not store them together? (define a private struct, return 2 values as you already do).

pkg/local_object_storage/metabase/inhume.go

fyrchik (Migrated from github.com) commented

2023-02-10 10:58:15 +00:00

It is local to the package, let's at least make it private. An I see it used only once, maybe we do not need a function at all?

fyrchik (Migrated from github.com) commented

2023-02-10 11:00:54 +00:00

Can we remove this comment? It is (was) misleading logical counter shouldn't increase at all on removal, what we increase is some other counter.

Can we remove this comment? It is (was) misleading `logical counter` shouldn't increase at all on removal, what we increase is some other counter.

pkg/local_object_storage/metabase/inhume.go

fyrchik (Migrated from github.com) commented

2023-02-10 11:01:17 +00:00

Why have you decided to have it outside if?

Why have you decided to have it outside `if`?

ir0nbee (Migrated from github.com) reviewed 2023-02-10 11:24:52 +00:00

CHANGELOG.md

					
				@ -11,6 +11,7 @@ Changelog for FrostFS Node

				- Add command `frostfs-adm morph netmap-candidates` (#1889)

				### Changed

				- Change `frostfs_node_engine_container_size` to counting sizes of logical objects

ir0nbee (Migrated from github.com) commented

2023-02-10 11:24:52 +00:00

Change {subject} to {doing something}. Looks correct to me.

ir0nbee (Migrated from github.com) reviewed 2023-02-10 11:27:02 +00:00

pkg/local_object_storage/metabase/inhume.go

ir0nbee (Migrated from github.com) commented

2023-02-10 11:27:02 +00:00

It is inside the if now.

It is inside the `if` now.

ir0nbee (Migrated from github.com) reviewed 2023-02-10 11:33:20 +00:00

pkg/local_object_storage/metabase/inhume.go

ir0nbee (Migrated from github.com) commented

2023-02-10 11:33:20 +00:00

I mean, it always was inside the if. Do you mean something else?

I mean, it always was inside the `if`. Do you mean something else?

alexvanin (Migrated from github.com) approved these changes 2023-02-16 08:53:58 +00:00

fyrchik (Migrated from github.com) reviewed 2023-02-16 11:34:58 +00:00

pkg/local_object_storage/metabase/inhume.go

fyrchik (Migrated from github.com) commented

2023-02-16 06:21:34 +00:00

I mean you have if inGraveyardWithKey(targetKey, graveyardBKT, garbageBKT) == 0 { below and containerID is used only there. There are 2 related lines which are divided in this commit for some reason.

I mean you have `if inGraveyardWithKey(targetKey, graveyardBKT, garbageBKT) == 0 {` below and `containerID` is used only there. There are 2 related lines which are divided in this commit for some reason.

Sign in to join this conversation.

No reviewers

carpawell

fyrchik

alexvanin