-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Bump Jaeger dependency on badger to 1.6.0 and resolve breaking changes #1884
Bump Jaeger dependency on badger to 1.6.0 and resolve breaking changes #1884
Conversation
…anges. 1) DefaultOptions is now a function with path default passed in 2) Iterator now seems unhappy when you do it.Item() != null and expects to be checked against .Valid() Signed-off-by: Andrey Kravtsov <qwerty@uber.com>
Ooops, will revert the elastic ver bump |
Signed-off-by: Andrey Kravtsov <qwerty@uber.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@burmanm any concerns?
Gopkg.toml
Outdated
@@ -137,7 +137,7 @@ required = [ | |||
|
|||
[[constraint]] | |||
name = "github.com/dgraph-io/badger" | |||
version = "=1.5.3" | |||
version = "^1.6.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given breaking changes, are you sure we want ^ and not =?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call, will fix
Signed-off-by: Andrey Kravtsov <qwerty@uber.com>
Codecov Report
@@ Coverage Diff @@
## master #1884 +/- ##
==========================================
- Coverage 98.44% 98.44% -0.01%
==========================================
Files 197 197
Lines 9645 9643 -2
==========================================
- Hits 9495 9493 -2
Misses 114 114
Partials 36 36
Continue to review full report at Codecov.
|
@@ -89,16 +88,14 @@ func (f *Factory) InitFromViper(v *viper.Viper) { | |||
func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) error { | |||
f.logger = logger | |||
|
|||
opts := badger.DefaultOptions | |||
opts.TableLoadingMode = options.MemoryMap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this one no longer needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should have mentioned. In 1.6.0 TableLoadingMode is defaulted to MemoryMap
Yeah, 1.6.0 is slower and has couple of new bugs I didn't want. So I would skip this version (that's why I've never sent a PR). |
Which problem is this PR solving?
Short description of the changes