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.

Sign up for GitHub

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

Upgrade go-msgpack to v2 2.1.1 #38

Merged
merged 3 commits into from
Nov 15, 2023
Merged

Upgrade go-msgpack to v2 2.1.1 #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ea49498
Upgrade go-msgpack to v2 2.1.1
Oct 19, 2023
4cd9b96
Clarify MsgpackUseNewTimeFormat
Oct 24, 2023
c696b73
Bump raft to 1.6.0 in v2
Nov 15, 2023
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
17 changes: 13 additions & 4 deletions v2/bolt_store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"os"
"time"

metrics "github.com/armon/go-metrics"
"github.com/armon/go-metrics"
v1 "github.com/boltdb/bolt"
"github.com/hashicorp/raft"
"go.etcd.io/bbolt"
Expand Down Expand Up @@ -36,6 +36,8 @@ type BoltStore struct {

// The path to the Bolt database file
path string

msgpackUseNewTimeFormat bool
}

// Options contains all the configuration used to open the Bbolt
Expand All @@ -51,6 +53,12 @@ type Options struct {
// write to the log. This is unsafe, so it should be used
// with caution.
NoSync bool

// MsgpackUseNewTimeFormat when set to true, force the underlying msgpack
// codec to use the new format of time.Time when encoding (used in
// go-msgpack v1.1.5 by default). Decoding is not affected, as all
// go-msgpack v2.1.0+ decoders know how to decode both formats.
MsgpackUseNewTimeFormat bool
Copy link
Member

Choose a reason for hiding this comment

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

If this is the format from 0.5.5 and before, shouldn't this be MsgpackUse Old TimeFormat? I swear I'm not trying to be pedantic and am genuinely confused. 😅

Copy link
Member

@mkeeler mkeeler Oct 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

MsgpackUseNewTimeFormat being true will cause TimeNotBuiltin to be set to false when configuring the msgpack Codec.

TimeNotBuiltin forces the msgpack library to not treat time as a builtin but instead rely on the binary marshaller (or text marshaller interfaces according to the docs).

I think the behavior is correct and also that having the zero value here cause disabling of the not-backwards-compatible time encoding is best so that we don't break Consul or Nomad.

I do think that the comment about the field is a little off as it sort of says that it causes the newer format to be used and that the format would be the default in versions less than 0.5.5. Should it be versions greater than 0.5.5?

Copy link
Author

Choose a reason for hiding this comment

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

@mkeeler is correct. I got the comment slightly wrong, and will correct it.

It's confusing due to the double-negative and trying to work with the default-false behavior of Go.

}

// readOnly returns true if the contained bolt options say to open
Expand All @@ -76,8 +84,9 @@ func New(options Options) (*BoltStore, error) {

// Create the new store
store := &BoltStore{
conn: handle,
path: options.Path,
conn: handle,
path: options.Path,
msgpackUseNewTimeFormat: options.MsgpackUseNewTimeFormat,
}

// If the store was opened read-only, don't try and create buckets
Expand Down Expand Up @@ -188,7 +197,7 @@ func (b *BoltStore) StoreLogs(logs []*raft.Log) error {
batchSize := 0
for _, log := range logs {
key := uint64ToBytes(log.Index)
val, err := encodeMsgPack(log)
val, err := encodeMsgPack(log, b.msgpackUseNewTimeFormat)
if err != nil {
return err
}
Expand Down
23 changes: 18 additions & 5 deletions v2/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,25 @@
module github.com/hashicorp/raft-boltdb/v2

go 1.16
go 1.20

require (
github.com/armon/go-metrics v0.0.0-20190430140413-ec5e00d3c878
github.com/armon/go-metrics v0.4.1
github.com/boltdb/bolt v1.3.1
github.com/hashicorp/go-msgpack v0.5.5
github.com/hashicorp/raft v1.1.0
github.com/hashicorp/raft-boltdb v0.0.0-20210409134258-03c10cc3d4ea
github.com/hashicorp/go-msgpack/v2 v2.1.1
github.com/hashicorp/raft v1.6.0
github.com/hashicorp/raft-boltdb v0.0.0-20230125174641-2a8082862702
go.etcd.io/bbolt v1.3.5
)

require (
github.com/fatih/color v1.13.0 // indirect
github.com/hashicorp/go-hclog v1.5.0 // indirect
github.com/hashicorp/go-immutable-radix v1.0.0 // indirect
github.com/hashicorp/go-msgpack v0.5.5 // indirect
github.com/hashicorp/golang-lru v0.5.0 // indirect
github.com/mattn/go-colorable v0.1.12 // indirect
github.com/mattn/go-isatty v0.0.14 // indirect
golang.org/x/sys v0.13.0 // indirect
)

replace github.com/hashicorp/raft => /Users/swenson/projects/raft
Loading