[#70] Add arrays of copies numbers for location constraints #99
Labels
No labels
P0
P1
P2
P3
good first issue
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: TrueCloudLab/frostfs-s3-gw#99
Loading…
Reference in a new issue
No description provided.
Delete branch "ironbee/frostfs-s3-gw:add-arrays-for-copies-numbers"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Signed-off-by: Artem Tataurov a.tataurov@yadro.com
3c3a53b925
to7675c938ed
WIP: Add arrays of copies numbers for location constraintsto Add arrays of copies numbers for location constraintsPlease fix commit message and link this PR to appropriate issue
@ -0,0 +7,4 @@
)
func TestCopiesNumberPicker(t *testing.T) {
Unnecessary empty line
@ -154,0 +170,4 @@
if len(vector) == 0 {
l.Warn("skipping the empty vector of copies numbers", zap.Int("index", i))
continue
Why do we use this? Probably we can use
break
(the same as for empty constraint)My thoughts were:
I will add the break.
@ -83,6 +83,7 @@ const ( // Settings.
// Policy.
cfgPolicyDefault = "placement_policy.default"
cfgPolicyRegionMapFile = "placement_policy.region_mapping"
cfgCopiesNumbers = "placement_policy.copies_numbers"
This new config param we should mention in
docs/configuration.md
and in config examples@ -81,0 +110,4 @@
copiesNumbersSplit := strings.Split(copiesNumbersStr, ",")
for i := range copiesNumbersSplit {
item := strings.ReplaceAll(copiesNumbersSplit[i], " ", "")
Can we drop this? It seems we can require that header must have format
1,2,3
Is there a validation somewhere or we need to implement it?
My suggestion was accepting only headers without spaces between numbers (
X-Amz-Meta-Frostfs-Copies-Number: 1,2,3
rather thanX-Amz-Meta-Frostfs-Copies-Number: 1, 2, 3
). But if we support both it's okWe support both cases, yes.
@ -0,0 +40,4 @@
})
t.Run("pick copies number from metadata", func(t *testing.T) {
metadata["frostfs-copies-number"] = "7, 8, 9"
We should also test
1,2,3
(without spaces)@ -32,6 +35,7 @@ type (
DefaultMaxAge int
NotificatorEnabled bool
CopiesNumber uint32
Let's rename this to
DefaultCopiesNumbers
and make it[]uint32
@ -150,3 +150,3 @@
}
p.CopiesNumber, err = getCopiesNumberOrDefault(p.Header, h.cfg.CopiesNumber)
copiesNumbers, err := h.pickCopiesNumbers(p.Header, bktInfo.LocationConstraint)
We can use:
@ -54,2 +52,4 @@
Reader: r.Body,
}
copiesNumbers, err := h.pickCopiesNumbers(parseMetadata(r), bktInfo.LocationConstraint)
The same
@ -171,0 +177,4 @@
Encryption: encryptionParams,
}
copiesNumbers, err := h.pickCopiesNumbers(metadata, dstBktInfo.LocationConstraint)
The same
@ -121,3 +121,3 @@
CopiesNumber: h.cfg.CopiesNumber,
}
copiesNumbers, err := h.pickCopiesNumbers(parseMetadata(r), bktInfo.LocationConstraint)
The same
@ -235,2 +227,4 @@
Encryption: encryptionParams,
}
copiesNumbers, err := h.pickCopiesNumbers(metadata, bktInfo.LocationConstraint)
The same
@ -146,3 +146,3 @@
CopiesNumber: h.cfg.CopiesNumber,
}
copiesNumbers, err := h.pickCopiesNumbers(parseMetadata(r), bktInfo.LocationConstraint)
The same
@ -226,0 +230,4 @@
NewLock: lock,
}
copiesNumbers, ok := h.cfg.CopiesNumbers[bktInfo.LocationConstraint]
Why don't we use
h.pickCopiesNumbers
?7675c938ed
to788096e4a1
788096e4a1
toe3d972e501
copies number
parameter for specific location constraintscopies number
parameter for specific location constraintsAdd arrays of copies numbers for location constraintsto [#70] Add arrays of copies numbers for location constraints@ -154,0 +164,4 @@
vector32[i] = uint32(vector[i])
}
if constraint == "" {
We can write:
@ -128,2 +128,4 @@
# Path to container policy mapping. The same as '--container-policy' flag for authmate
region_mapping: /path/to/container/policy.json
# Array of locations constraints and their vectors of copies numbers
copies_numbers:
Let's add the same example to
config/config.env
@ -288,0 +285,4 @@
|------------------|--------------------------------------------------|---------------|---------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `default` | `string` | yes | `REP 3` | Default policy of placing containers in FrostFS. If a user sends a request `CreateBucket` and doesn't define policy for placing of a container in FrostFS, the S3 Gateway will put the container with default policy. |
| `region_mapping` | `string` | yes | | Path to file that maps aws `LocationContraint` values to FrostFS placement policy. The similar to `--container-policy` flag in `frostfs-s3-authmate` util, see in [docs](./authmate.md#containers-policy) |
| `copies_numbers` | `[]`[Copies numbers](#copies_numbers-subsection) | no | | Array of configured location constraints and their copies numbers. |
Can we provide some example above, like:
@ -0,0 +7,4 @@
)
func TestCopiesNumberPicker(t *testing.T) {
var locCosntraints = map[string][]uint32{}
Typo. Should be
var locConstraints = ...
@ -0,0 +26,4 @@
metadata["somekey1"] = "5, 6, 7"
expectedCopiesNumbers := []uint32{1}
actualCopiesNumbers, _ := h.pickCopiesNumbers(metadata, locationConstraint2)
Why don't we check error?
e3d972e501
to8286fe27ce
8286fe27ce
to07ae24b97d
07ae24b97d
to8af2432415
8af2432415
to438ba1ab58
@ -154,0 +157,4 @@
for i := 0; ; i++ {
key := cfgCopiesNumbers + "." + strconv.Itoa(i) + "."
constraint := v.GetString(key + "location_constraint")
vector := v.GetIntSlice(key + "vector")
It seems here we should use
v.GetStringSlice
otherwise we cannot properly get vector if it passed via environment variables438ba1ab58
to983d77a9c8
@ -154,0 +160,4 @@
vector := v.GetStringSlice(key + "vector")
vector32 := make([]uint32, len(vector))
for i := range vector {
Let's use
j
to avoid confusing withi
from outer loop@ -154,0 +164,4 @@
parsedValue, err := strconv.ParseUint(vector[i], 10, 32)
if err != nil {
l.Error("cannot parse copies numbers", zap.Error(err))
break
Actually, this breaks only inner loop, but this isn't enough.
We should use
break LOOP
whereLOOP
is a label right above the outer loop.983d77a9c8
toe487ee5b7d
copies number
parameter for specific location constraints #70