minio / mc

Unix like utilities for object store
https://min.io/download
GNU Affero General Public License v3.0
2.86k stars 548 forks source link

fix replication bandwidth unit conversion #4922

Closed kanagarajkm closed 5 months ago

kanagarajkm commented 5 months ago

Community Contribution License

All community contributions in this pull request are licensed to the project maintainers under the terms of the Apache 2 license. By creating this pull request I represent that I have the right to license the contributions to the project maintainers under the Apache 2 license.

Description

the user input will be in bytes, need not convert it again to bytes.

Motivation and Context

To avoid any confusion while setting the replication bandwidth.

How to test this PR?

./mc replicate update ALIAS/BUCKET --id site-repl-769b7d7a-b0df-4d66-abf1-68516ddb53d4 --bandwidth 1G --remote-bucket REMOTE_ALIAS/BUCKET

Types of changes

Checklist:

poornas commented 5 months ago

@kanagarajkm , the bandwidth is converted here to bytes because the server is accepting in bytes/sec

kanagarajkm commented 5 months ago

@poornas https://pkg.go.dev/github.com/dustin/go-humanize#ParseBytes is already returning the value in bytes, it need not be converted again

poornas commented 5 months ago

@poornas https://pkg.go.dev/github.com/dustin/go-humanize#ParseBytes is already returning the value in bytes, it need not be converted again

@kanagarajkm , the bandwidth setting is supposed to be set in bits/sec - whereas the humanize package does not support conversion of bits. Therefore the parse followed by /8 to convert to bytes.

What we can do instead is, take bandwidth only in bytes (flags need to be updated in replicate add/update), then your change will be ok

kanagarajkm commented 5 months ago

What we can do instead is, take bandwidth only in bytes (flags need to be updated in replicate add/update), then your change will be ok

@poornas I have updated the flag usage.

Also removed the bits conversion from mc replicate status and mc admin replicate info. PTAL

harshavardhana commented 5 months ago

Bytes are actually OK; that is how we communicate anyway. Its not much to talk about things in bits anyway.