Feat/Object count metrics #1712

carpawell · 2022-08-19T17:34:55Z

Related to #1658

# HELP neofs_node_object_counter Objects counters per shards
# TYPE neofs_node_object_counter gauge
neofs_node_object_counter{shard="A1AJ1cgEdGGYC2kGTGQxQJ"} 5
neofs_node_object_counter{shard="S9b69efduvCprPU4FWKirH"} 3

Does not contain the node's total object count since on the shard level it is quite easy to control objects but on the SE level it is not always clear whether a shard contains an object or not:

GC works on the shard level
SE's Delete always calls shard's Inhume not Delete

It is possible to solve (e.g. some SE'e atomic counter that could be passed to every shard) but IMO it is easier to calculate on a metric collector side.

codecov · 2022-08-19T17:38:48Z

Codecov Report

Merging #1712 (584297a) into master (61f0d85) will increase coverage by 0.09%.
The diff coverage is 53.38%.

❗ Current head 584297a differs from pull request most recent head 9c4c7b9. Consider uploading reports for the commit 9c4c7b9 to get more accurate results

@@            Coverage Diff             @@
##           master    #1712      +/-   ##
==========================================
+ Coverage   32.50%   32.60%   +0.09%     
==========================================
  Files         337      338       +1     
  Lines       22701    22820     +119     
==========================================
+ Hits         7380     7441      +61     
- Misses      14708    14754      +46     
- Partials      613      625      +12

Impacted Files	Coverage Δ
pkg/local_object_storage/engine/metrics.go	`0.00% <ø> (ø)`
pkg/local_object_storage/engine/shards.go	`66.29% <0.00%> (-13.44%)`	⬇️
pkg/local_object_storage/shard/shard.go	`61.46% <16.66%> (-8.87%)`	⬇️
pkg/local_object_storage/metabase/iterators.go	`67.08% <46.66%> (-4.79%)`	⬇️
pkg/local_object_storage/metabase/control.go	`77.94% <50.00%> (-1.75%)`	⬇️
pkg/local_object_storage/metabase/put.go	`74.00% <50.00%> (-0.36%)`	⬇️
pkg/local_object_storage/metabase/delete.go	`76.92% <70.37%> (-0.55%)`	⬇️
pkg/local_object_storage/metabase/counter.go	`73.91% <73.91%> (ø)`
pkg/local_object_storage/shard/control.go	`79.06% <100.00%> (+0.16%)`	⬆️
pkg/local_object_storage/shard/delete.go	`69.04% <100.00%> (+0.75%)`	⬆️
... and 2 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

realloc · 2022-08-20T06:48:49Z

Agree, the total node's object count can be handled externally.

Is it possible to add a separate per shard counter for logically available objects? For example, some objects may be inhumed, but not physically removed from the graveyard yet, so we could see the difference in the monitoring system.

carpawell · 2022-08-22T13:02:08Z

Is it possible to add a separate per shard counter for logically available objects?

@realloc, that PR contains the metrics with physically stored objects (no split objects, tombstoned objects are included in the counter if they are not deleted by a shard yet). Do you mean to just add another counter that means the same but excludes objects that have TS/GC?

realloc · 2022-08-22T13:21:37Z

Yes, there should be one counter with all types of objects available and one for all types of objects physically stored.

cthulhu-rider · 2022-08-22T10:40:00Z

pkg/local_object_storage/metabase/counter.go

+// tracked since it was opened and initialized.
+//
+// Returns only the errors that do not allow reading counter
+// in bbolt database.


cthulhu-rider · 2022-08-22T10:40:34Z

pkg/local_object_storage/metabase/counter.go

+	return
+}
+
+// updateCounter updates the object counter. Must be called NOT for read-only


Suggested change

// updateCounter updates the object counter. Must be called NOT for read-only

// updateCounter updates the object counter. Tx MUST be writable.

cthulhu-rider · 2022-08-22T10:45:10Z

pkg/local_object_storage/metabase/counter.go

+// If inc == `true`, increases the counter, decreases otherwise.
+func (db *DB) updateCounter(tx *bbolt.Tx, delta uint64, inc bool) error {
+	b := tx.Bucket(shardInfoBucket)
+	if b != nil {


immediate return on nil for smaller if?

cthulhu-rider · 2022-08-22T10:46:19Z

pkg/local_object_storage/metabase/counter.go

+	b := tx.Bucket(shardInfoBucket)
+	if b != nil {
+		var counter uint64
+		newCounter := make([]byte, 8)


Alloc on miss only?

cthulhu-rider · 2022-08-22T10:46:39Z

pkg/local_object_storage/metabase/counter.go

+			counter = binary.LittleEndian.Uint64(data)
+		case 0:
+		default:
+			return nil


Is it normal? Maybe panic?

well... ok, added panic

@fyrchik, ping

cthulhu-rider · 2022-08-22T10:47:26Z

pkg/local_object_storage/metabase/counter.go

+		if inc {
+			counter += delta
+		} else {
+			if counter <= delta {


Merge with else?

merged but usually, i prefer not to use else if

cthulhu-rider · 2022-08-22T10:57:00Z

pkg/local_object_storage/metabase/delete.go

 		}
 	}

+	err := db.updateCounter(tx, rawDeleted, false)
+	if err != nil {
+		return 0, fmt.Errorf("could not decrease object counter: %w", err)


Maybe make it a pure feature and not fail the original op?

Same for PUT.

agree, removed return, added error log

pkg/local_object_storage/metabase/VERSION.md

pkg/local_object_storage/metabase/counter_test.go

fyrchik · 2022-08-24T09:24:27Z

pkg/local_object_storage/metabase/counter.go

+	switch len(data) {
+	case 8:
+		counter = binary.LittleEndian.Uint64(data)
+	case 0:
+	default:
+		panic(fmt.Errorf(
+			"unexpected len of object counter value: %d", len(data)),
+		)
+	}


I think just checking len(data) == 8 would be enough.

fyrchik · 2022-08-24T09:25:43Z

pkg/local_object_storage/metabase/delete.go

+		db.log.Error("could not decrease object counter",
+			zap.Error(err))


No, let's not log anything inside of a transaction. It is potentially a blocking call.

missed that it is inside a bolt TX

return back the returned code: no logging, direct err to a caller only

fyrchik · 2022-08-24T09:26:45Z

pkg/local_object_storage/metabase/put.go

+		if err != nil {
+			db.log.Error("could not increase object counter: %w",
+				zap.Error(err))


Here too. I think we can (should) return the error to the caller, updateCounter has no logical errors.

fyrchik · 2022-08-24T09:27:28Z

pkg/metrics/object.go

@@ -254,3 +270,15 @@ func (m objectServiceMetrics) AddPutPayload(ln int) {
 func (m objectServiceMetrics) AddGetPayload(ln int) {
 	m.getPayload.Add(float64(ln))
 }
+
+func (m objectServiceMetrics) ChangeBy(shardID string, delta int) {


Change what? Could you add ObjectCounter to the name?

removed, only AddToObjectCounter(shardID string, delta int) and SetObjectCounter(shardID string, v uint64) are kept on the metrics side; wrappers still have Inc*, Dec* methods

fyrchik · 2022-08-24T09:31:43Z

pkg/metrics/object.go

+func (m objectServiceMetrics) IncObjectCounter(shardID string) {
+	m.shardMetrics.With(prometheus.Labels{shardIDLabelKey: shardID}).Inc()
+}
+
+func (m objectServiceMetrics) DecObjectCounter(shardID string) {
+	m.shardMetrics.With(prometheus.Labels{shardIDLabelKey: shardID}).Dec()
+}


Why did you decide to stay on 3 methods instead of a single AddToObjectCounter which covers both inc and dec?

kept only two: Set* and Add*

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>

Increment shard's object counter on successful `Put` calls and decrement on `Delete`. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>

Includes: 1. Renaming counter key to distinguish logical and physical objects 2. Version update dropping since changes could be done in a compatible way Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>

carpawell added the neofs-storage Storage node application issues label Aug 19, 2022

carpawell self-assigned this Aug 19, 2022

carpawell marked this pull request as ready for review August 19, 2022 17:53

carpawell requested review from fyrchik, cthulhu-rider, anatoly-bogatyrev and realloc August 19, 2022 17:54

cthulhu-rider reviewed Aug 23, 2022

View reviewed changes

fyrchik reviewed Aug 23, 2022

View reviewed changes

pkg/local_object_storage/metabase/VERSION.md Outdated Show resolved Hide resolved

pkg/local_object_storage/metabase/counter_test.go Show resolved Hide resolved

fyrchik reviewed Aug 24, 2022

View reviewed changes

carpawell added 6 commits August 25, 2022 15:56

[#1658] meta: Add object counter

b51cc5c

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>

[#1658] metrics: Add object counter metric

07d8784

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>

[#1658] node: Support object counter metrics

14890f6

Increment shard's object counter on successful `Put` calls and decrement on `Delete`. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>

[#1658] node: Read metrics from meta on startup

a4595d7

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>

[#1658] Update CHANGELOG

9742349

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>

[#1658] node: Init object counter on meta's Init

9c4c7b9

Includes: 1. Renaming counter key to distinguish logical and physical objects 2. Version update dropping since changes could be done in a compatible way Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>

carpawell requested review from fyrchik and cthulhu-rider August 25, 2022 15:59

fyrchik approved these changes Aug 25, 2022

View reviewed changes

fyrchik merged commit c7c1c25 into nspcc-dev:master Aug 25, 2022

carpawell deleted the feat/object-count-metrics branch August 25, 2022 16:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/Object count metrics #1712

Feat/Object count metrics #1712

carpawell commented Aug 19, 2022 •

edited

Loading

codecov bot commented Aug 19, 2022 •

edited

Loading

realloc commented Aug 20, 2022

carpawell commented Aug 22, 2022 •

edited

Loading

realloc commented Aug 22, 2022

cthulhu-rider Aug 22, 2022

cthulhu-rider Aug 22, 2022

cthulhu-rider Aug 22, 2022

cthulhu-rider Aug 22, 2022

cthulhu-rider Aug 22, 2022

carpawell Aug 23, 2022

cthulhu-rider Aug 22, 2022

carpawell Aug 23, 2022

cthulhu-rider Aug 22, 2022

carpawell Aug 23, 2022

fyrchik Aug 24, 2022

fyrchik Aug 24, 2022

carpawell Aug 25, 2022

fyrchik Aug 24, 2022

fyrchik Aug 24, 2022

carpawell Aug 25, 2022

fyrchik Aug 24, 2022

carpawell Aug 25, 2022

	// updateCounter updates the object counter. Must be called NOT for read-only
	// updateCounter updates the object counter. Tx MUST be writable.

		db.log.Error("could not decrease object counter",
		zap.Error(err))

Feat/Object count metrics #1712

Feat/Object count metrics #1712

Conversation

carpawell commented Aug 19, 2022 • edited Loading

codecov bot commented Aug 19, 2022 • edited Loading

Codecov Report

realloc commented Aug 20, 2022

carpawell commented Aug 22, 2022 • edited Loading

realloc commented Aug 22, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carpawell commented Aug 19, 2022 •

edited

Loading

codecov bot commented Aug 19, 2022 •

edited

Loading

carpawell commented Aug 22, 2022 •

edited

Loading