dgraph-io / badger

Fast key-value DB in Go.
https://dgraph.io/badger
Apache License 2.0
13.73k stars 1.17k forks source link

[BUG]: ristretto uses glog which breaks a lot of applications #1922

Open mkevac opened 1 year ago

mkevac commented 1 year ago

What version of Badger are you using?

v4.0.1

What version of Go are you using?

Not relevant

Have you tried reproducing the issue with the latest release?

Yes

What is the hardware spec (RAM, CPU, OS)?

Not relevant

What steps will reproduce the bug?

Use badger in an app that has -v flag.

Expected behavior and actual result.

Expected: program does not panic. Actual: program panics.

 marko@daemons3  ~/foobar   badger ±  ./foobar
./foobar flag redefined: v
panic: ./foobar flag redefined: v

goroutine 1 [running]:
flag.(*FlagSet).Var(0xc000168120, {0x1943150, 0x2396f01}, {0x1939880, 0x1}, {0x17ec0d9, 0xd})
    /home/marko/go/src/flag/flag.go:1005 +0x2c5
flag.BoolVar(...)
    /home/marko/go/src/flag/flag.go:732
foobar/service.initialize({0x17eeb64, 0x10}, {0x1774800, 0xc00013c660}, {0x0, 0x0}, 0x1850638)
    /home/marko/pkg/mod/foobar@v4.0.20/service/service.go:326 +0x3a5
foobar/service.Initialize(...)
    /home/marko/pkg/mod/foobar@v4.0.20/service/service.go:302
main.main()
    /home/marko/foobar/main.go:42 +0x331

This is because glog library pollutes flags and adds -v flag. This breaks every program that has -v flag. Library should not pollute global flags.

Dependency graph

 go mod why github.com/golang/glog
# github.com/golang/glog
foobar/ukv/db
github.com/dgraph-io/badger/v4
github.com/dgraph-io/ristretto/z
github.com/golang/glog

See relevant issues: https://github.com/DataDog/dd-trace-go/issues/1153 https://github.com/dgraph-io/ristretto/pull/293

Additional information

Ristretto is abandoned and they will not remove glog dependency. Consider moving to ristretto fork.

David-Mao commented 1 year ago

Gentle ping. This is quite annoying, and a fix seems not that complicated (as long as someone can commit https://github.com/dgraph-io/ristretto/pull/293 )

mholt commented 1 year ago

Less gentle ping. :innocent:

This package is a 4th-degree dependency of my application. I'm surprised to see CLI flags and panics that are out of my control.

This is bizarre to me as glog even states in its README that it's only for internal use at Google (why it is even public is beyond me):

The repository contains an open source version of the log package used inside Google. The master copy of the source lives inside Google, not here. The code in this repo is for export only and is not itself under development. Feature requests will be ignored.

Can we at least get some acknowledgement from @dgraph-io that this dependency will be removed?

mangalaman93 commented 1 year ago

Ristretto has a change merged that removes glog. As soon as we do a new release, we should upgrade ristretto in badger.

Lercher commented 11 months ago

Just for the sake of better "full-text findability". Besides -v glog also silently adds these command line flags to programs using badger v4

  -alsologtostderr
        log to standard error as well as files
  -log_backtrace_at value
        when logging hits line file:N, emit a stack trace
  -log_dir string
        If non-empty, write log files in this directory
  -logtostderr
        log to standard error instead of files
  -stderrthreshold value
        logs at or above this threshold go to stderr
  -v value
        log level for V logs
  -vmodule value
        comma-separated list of pattern=N settings for file-filtered logging
pluja commented 9 months ago

Will this be addressed? I'd really like to get rid of the extra flags that are being added by using badger v4... Is there any known way to remove them?

EDIT: I found a way, you can declare your own flag set with flag.NewFlagSet("flagsetname", flag.ExitOnError) and use that instead of default...

github-actions[bot] commented 1 month ago

This issue has been stale for 60 days and will be closed automatically in 7 days. Comment to keep it open.

mholt commented 1 month ago

Please keep it open.