Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

replication: change metrics API #1833

Merged
merged 2 commits into from Jun 28, 2023
Merged

Conversation

poornas
Copy link
Contributor

@poornas poornas commented May 26, 2023

for additional info to be passed back.

Copy link
Member

@vadmeste vadmeste left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few comments

pkg/replication/replication.go Outdated Show resolved Hide resolved
pkg/replication/replication.go Outdated Show resolved Hide resolved
pkg/replication/replication.go Outdated Show resolved Hide resolved
pkg/replication/replication.go Outdated Show resolved Hide resolved
@klauspost
Copy link
Contributor

klauspost commented May 29, 2023

@poornas Instead of trying to do many small corrections here is my proposal:

// XferStats holds transfer rate for uploads
type XferStats struct {
	// Totals
	Count int64 `json:"count"`
	Bytes int64 `json:"bytes"`

	// Averages - 1 minute?
	AvgBytes  float64 `json:"avgBytes"`
	PeakBytes float64 `json:"peakBytes"`
	CurrBytes float64 `json:"currBytes"`

	AvgCount  float64 `json:"avgCount"`
	PeakCount float64 `json:"peakCount"`
	CurrCount float64 `json:"currCount"`
}

type NodeQueueStats struct {
	Name          string `json:"name"`
	Uptime        int64  `json:"uptime"`
	ActiveWorkers int    `json:"activeWorkers"`
	QueuedCount   int    `json:"queuedCount"`
	QueuedBytes   int64  `json:"queuedBytes"`

	// Transfer statistics For objects larger/smaller than XXXKB.
	Large XferStats `json:"large"`
	Small XferStats `json:"small"`
}

type QueueStats struct {
	Nodes []NodeQueueStats `json:"nodes"`
}
  • Groups all small/large values.
  • Cuts out "replication" and "repl" from names.
  • Adds per node uptime.
  • Adds "count" rates.

Still needs more docs.

for additional info to be passed back.
@poornas
Copy link
Contributor Author

poornas commented May 30, 2023

@poornas Instead of trying to do many small corrections here is my proposal:

Thanks @klauspost , adopted some of the suggestions - the xfer stats refer to rate/sec and different from the in-queue stats, so kept it separate.

@klauspost
Copy link
Contributor

Running lint check
Error: pkg/replication/replication.go:770:6: exported: exported type ReplQNodeStats should have comment or be unexported (revive)
type ReplQNodeStats struct {
     ^
Error: pkg/replication/replication.go:784:6: exported: exported type ReplQueueStats should have comment or be unexported (revive)
type ReplQueueStats struct {
     ^
Error: pkg/replication/replication.go:788:1: exported: exported method ReplQueueStats.Workers should have comment or be unexported (revive)
func (q ReplQueueStats) Workers() int64 {
^
Error: pkg/replication/replication.go:796:6: exported: exported type ReplQStats should have comment or be unexported (revive)
type ReplQStats struct {
     ^
Error: pkg/replication/replication.go:808:1: exported: exported method ReplQueueStats.QStats should have comment or be unexported (revive)
func (q ReplQueueStats) QStats() (r ReplQStats) {
^
Error: pkg/replication/replication.go:844:6: exported: exported type MetricsV2 should have comment or be unexported (revive)
type MetricsV2 struct {
     ^
Error: make: *** [Makefile:15: lint] Error 1

(also some Repl prefixes left).

Copy link
Contributor

@klauspost klauspost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

api-bucket-replication.go Outdated Show resolved Hide resolved
api-bucket-replication.go Outdated Show resolved Hide resolved
api-bucket-replication.go Outdated Show resolved Hide resolved
@harshavardhana
Copy link
Member

@poornas PTAL

Copy link
Member

@vadmeste vadmeste left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few comments

pkg/replication/replication.go Outdated Show resolved Hide resolved
pkg/replication/replication.go Outdated Show resolved Hide resolved
pkg/replication/replication.go Outdated Show resolved Hide resolved
pkg/replication/replication.go Outdated Show resolved Hide resolved
@harshavardhana harshavardhana merged commit ac95c83 into minio:master Jun 28, 2023
7 checks passed
poornas added a commit to poornas/minio-go that referenced this pull request Jul 19, 2023
poornas added a commit to poornas/minio-go that referenced this pull request Jul 19, 2023
harshavardhana pushed a commit that referenced this pull request Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants