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

Integrated Storage (Raft): Add Support for max_entry_size Config #9027

Merged
merged 27 commits into from
Jun 1, 2020
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
f44d7fd
Add kvMaxValueSize field the raft storage backend
alexanderbez May 18, 2020
d8e4680
Add context to error
alexanderbez May 18, 2020
67726b5
Return error in RaftBackend#Put for value that is too large
alexanderbez May 18, 2020
ba3ce52
Add physical.ErrValueTooLarge
alexanderbez May 19, 2020
2f101d2
Add TestRaft_Backend_LargeValue
alexanderbez May 19, 2020
808271d
Add case for Get after failed put
alexanderbez May 19, 2020
0d6f513
Add value size check to RaftBackend#applyLog
alexanderbez May 19, 2020
366721b
Implement TestRaft_TransactionalBackend_LargeValue
alexanderbez May 19, 2020
c2fd553
Add kv_max_value_size section to raft storage doc
alexanderbez May 19, 2020
b279a90
Add warn log to Get
alexanderbez May 19, 2020
93318b9
update log
alexanderbez May 19, 2020
9f4fea4
Use fatal instead of error
alexanderbez May 19, 2020
9c5f0dc
Merge branch 'master' into bez/raft-max-kv-value-size
alexanderbez May 19, 2020
30a5b96
Merge branch 'master' into bez/raft-max-kv-value-size
alexanderbez May 20, 2020
77dc11e
cl++
alexanderbez May 20, 2020
5a3e7ff
Merge branch 'bez/raft-max-kv-value-size' of github.com:hashicorp/vau…
alexanderbez May 20, 2020
2f5913d
Change config value
alexanderbez May 20, 2020
2d2b9f9
Update godoc
alexanderbez May 20, 2020
ee34ada
Merge branch 'master' into bez/raft-max-kv-value-size
alexanderbez May 21, 2020
ee7f8d3
Update physical/raft/raft_test.go
alexanderbez May 26, 2020
bc63c8e
Update physical/raft/raft_test.go
alexanderbez May 26, 2020
5c71cfb
Change default to 2x
alexanderbez May 27, 2020
fcb4201
Add entry size sample metric
alexanderbez May 27, 2020
f42e2d3
Add metric to docs
alexanderbez May 28, 2020
80eef73
Update physical/raft/raft.go
alexanderbez May 29, 2020
f859f3a
Update entry_size references
alexanderbez May 29, 2020
a281836
cl++
alexanderbez May 29, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

CHANGES:

* storage/raft: The storage configuration now accepts a new `kv_max_value_size` config that will limit
the total size in bytes of any entry commited via raft. It defaults to `"1572864"` -- three times the chunking size. [[GH-9027](https://github.com/hashicorp/vault/pull/9027)]
* token: Token creation with custom token ID via `id` will no longer allow periods (`.`) as part of the input string.
The final generated token value may contain periods, such as the `s.` prefix for service token
indication. [[GH-8646](https://github.com/hashicorp/vault/pull/8646/files)]
Expand Down
66 changes: 55 additions & 11 deletions physical/raft/raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ var (
snapshotsRetained = 2

restoreOpDelayDuration = 5 * time.Second

defaultKVMaxValueSize = uint64(3 * raftchunking.ChunkSize)
)

// RaftBackend implements the backend interfaces and uses the raft protocol to
Expand Down Expand Up @@ -107,6 +109,10 @@ type RaftBackend struct {

// permitPool is used to limit the number of concurrent storage calls.
permitPool *physical.PermitPool

// kvMaxValueSize imposes a limit on values in individual KV operations. It is
// suggested to use a value of 3x the Raft chunking size for optimal performance.
kvMaxValueSize uint64
}

// LeaderJoinInfo contains information required by a node to join itself as a
Expand Down Expand Up @@ -226,6 +232,7 @@ func NewRaftBackend(conf map[string]string, logger log.Logger) (physical.Backend
var log raft.LogStore
var stable raft.StableStore
var snap raft.SnapshotStore

var devMode bool
if devMode {
store := raft.NewInmemStore()
Expand Down Expand Up @@ -303,16 +310,27 @@ func NewRaftBackend(conf map[string]string, logger log.Logger) (physical.Backend
}
}

kvMaxValueSize := defaultKVMaxValueSize
if kvMaxValueSizeCfg := conf["kv_max_value_size"]; len(kvMaxValueSizeCfg) != 0 {
i, err := strconv.Atoi(kvMaxValueSizeCfg)
if err != nil {
return nil, fmt.Errorf("failed to parse 'kv_max_value_size': %w", err)
}

kvMaxValueSize = uint64(i)
}

return &RaftBackend{
logger: logger,
fsm: fsm,
conf: conf,
logStore: log,
stableStore: stable,
snapStore: snap,
dataDir: path,
localID: localID,
permitPool: physical.NewPermitPool(physical.DefaultParallelOperations),
logger: logger,
fsm: fsm,
conf: conf,
logStore: log,
stableStore: stable,
snapStore: snap,
dataDir: path,
localID: localID,
permitPool: physical.NewPermitPool(physical.DefaultParallelOperations),
kvMaxValueSize: kvMaxValueSize,
}, nil
}

Expand Down Expand Up @@ -925,12 +943,31 @@ func (b *RaftBackend) Get(ctx context.Context, path string) (*physical.Entry, er
b.permitPool.Acquire()
defer b.permitPool.Release()

return b.fsm.Get(ctx, path)
entry, err := b.fsm.Get(ctx, path)
if entry != nil {
valueLen := len(entry.Value)
if uint64(valueLen) > b.kvMaxValueSize {
b.logger.Warn(
"retrieved entry value is too large; see https://www.vaultproject.io/docs/configuration/storage/raft#raft-parameters",
"size", valueLen, "suggested", b.kvMaxValueSize,
)
}
}

return entry, err
}

// Put inserts an entry in the log for the put operation
// Put inserts an entry in the log for the put operation. It will return an
// error if the value exceeds the configured kv_max_value_size or if the call to
// applyLog fails.
func (b *RaftBackend) Put(ctx context.Context, entry *physical.Entry) error {
defer metrics.MeasureSince([]string{"raft-storage", "put"}, time.Now())

valueLen := len(entry.Value)
if uint64(valueLen) > b.kvMaxValueSize {
return fmt.Errorf("%s; got %d bytes, max: %d bytes", physical.ErrValueTooLarge, valueLen, b.kvMaxValueSize)
calvn marked this conversation as resolved.
Show resolved Hide resolved
}

command := &LogData{
Operations: []*LogOperation{
&LogOperation{
Expand Down Expand Up @@ -1004,6 +1041,13 @@ func (b *RaftBackend) applyLog(ctx context.Context, command *LogData) error {
return errors.New("raft storage backend is not initialized")
}

for _, op := range command.Operations {
valueLen := len(op.Value)
if uint64(valueLen) > b.kvMaxValueSize {
return fmt.Errorf("%s; got %d bytes, max: %d bytes", physical.ErrValueTooLarge, valueLen, b.kvMaxValueSize)
}
}

commandBytes, err := proto.Marshal(command)
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return err
Expand Down
56 changes: 56 additions & 0 deletions physical/raft/raft_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
fmt "fmt"
"io"
"io/ioutil"
"math/rand"
"os"
"path/filepath"
"testing"
Expand Down Expand Up @@ -203,6 +204,61 @@ func TestRaft_Backend(t *testing.T) {
physical.ExerciseBackend(t, b)
}

func TestRaft_Backend_LargeValue(t *testing.T) {
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
b, dir := getRaft(t, true, true)
defer os.RemoveAll(dir)

t.Helper()
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved

value := make([]byte, defaultKVMaxValueSize+1)
rand.Read(value)
entry := &physical.Entry{Key: "foo", Value: value}

if err := b.Put(context.Background(), entry); err == nil {
calvn marked this conversation as resolved.
Show resolved Hide resolved
t.Fatal("expected error for put entry")
}
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved

out, err := b.Get(context.Background(), entry.Key)
if err != nil {
t.Fatalf("unexpected error after failed put: %v", err)
}
if out != nil {
t.Fatal("expected response entry to be nil after a failed put")
}
}

func TestRaft_TransactionalBackend_LargeValue(t *testing.T) {
b, dir := getRaft(t, true, true)
defer os.RemoveAll(dir)

t.Helper()
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved

value := make([]byte, defaultKVMaxValueSize+1)
rand.Read(value)

txns := []*physical.TxnEntry{
&physical.TxnEntry{
Operation: physical.PutOperation,
Entry: &physical.Entry{
Key: "foo",
Value: value,
},
},
}

if err := b.Transaction(context.Background(), txns); err == nil {
t.Fatal("expected error for transactions")
}

out, err := b.Get(context.Background(), txns[0].Entry.Key)
if err != nil {
t.Fatalf("unexpected error after failed put: %v", err)
}
if out != nil {
t.Fatal("expected response entry to be nil after a failed put")
}
}

func TestRaft_Backend_ListPrefix(t *testing.T) {
b, dir := getRaft(t, true, true)
defer os.RemoveAll(dir)
Expand Down
10 changes: 10 additions & 0 deletions website/pages/docs/configuration/storage/raft.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,16 @@ time. To use Raft for HA coordination users must also use Raft for storage.
still need to be unsealed manually. See the section below that describes the
parameters accepted by the `retry_join` stanza.

- `kv_max_value_size` `(integer: 1572864)` - This configures the maximum number of
bytes for a value in a Raft entry. It applies to both Put operations and transactions.
Any value size in an entry or transaction exceeding this configuration value
will cause the respective operation to fail. Raft has a suggested max size of
data in a raft log entry. This is based on current architecture, default timing,
etc. Integrated storage also uses a chunk size that is the threshold used for
breaking a large value into chunks. By default, the chunk size is the same as
raft's max size log entry. The default value for this configuration is 1572864
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
-- three times the chunking size.

### `retry_join` stanza

- `leader_api_addr` `(string: "")` - Address of a possible leader node.
Expand Down