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

Drop debug messages in kv_bolt.go from "warning" to "debug" level #83

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alaric-dotmesh
Copy link

Hi there folks!

Just a quick one - I found that bolt/kv_bolt.go was logging a load of debug messages at the "warning" level, which was making my logs look really messy :-D

I presume the logging is wanted for people hacking on this library, so this PR just demotes them from "warning" to "debug" level, so they can still be seen if you enable the debug level in logrus.

Hope it's useful to somebody other than just me! Thanks for kvdb!

@adityadani
Copy link
Member

adityadani commented Apr 12, 2019

@alaric-dotmesh Thanks for submitting a PR.

Currently the implementation for Bolt in kvdb, has not completely flushed out, which is why you see Debug messages getting logged. Just a word of caution, we will not recommend using it in production right now.

Also the unit tests are currently not running on bolt, because we have seen failures in different APIs. If you wish you can take a stab at it and try to fix them.

Btw, we actively use kvdb for other backend providers like etcd, consul and in-memory.

@codecov-io
Copy link

Codecov Report

Merging #83 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #83   +/-   ##
=======================================
  Coverage   68.96%   68.96%           
=======================================
  Files           7        7           
  Lines        3415     3415           
=======================================
  Hits         2355     2355           
+ Misses        826      825    -1     
- Partials      234      235    +1
Impacted Files Coverage Δ
etcd/v2/kv_etcd.go 68.07% <0%> (-0.33%) ⬇️
etcd/v3/kv_etcd.go 67.14% <0%> (+0.2%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 135442d...38e6995. Read the comment docs.

@lukemarsden
Copy link

lukemarsden commented May 10, 2019

Hi, I work with Alaric. We've seen issues where when Bolt + kvdb gets restarted, the ModifiedIndex values for new records get reset to zero. That's not consistent with how etcd works (ModifiedIndex values are monotically increasing across restarts), and so it's not possible to write portable code which is like "read everything of some type from the store, write it to memory, then find the maximum ModifiedIndex from that set of things, then start a watcher from that index which will catch anything that got added while we were reading" because on etcd, maxIdx needs to be max(idx of all records) but on bolt it needs to be 0 but if you feed that to etcd it chokes if it's ever compacted the index.

Any suggestions for how to fix this in the bolt backend for kvdb? Currently this is blocking our usage of bolt.

@adityadani
Copy link
Member

I totally agree with your suggested intended behavior for bolt with respect to ModifiedIndex. We will look into how to fix this in kvdb's boltDB implementation. Let us know if you have any suggestions on how this can be implemented in kvdb.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants