BlockIntervalSeconds metric & DB tests cleanup & benchmark improv. by melekes · Pull Request #2337 · tendermint/tendermint

melekes · 2018-09-05T13:10:13Z

DO NOT SQUASH

codecov-io · 2018-09-05T13:27:23Z

Codecov Report

Merging #2337 into develop will decrease coverage by 0.19%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           develop    #2337     +/-   ##
==========================================
- Coverage    61.08%   60.89%   -0.2%     
==========================================
  Files          197      197             
  Lines        16246    16297     +51     
==========================================
  Hits          9924     9924             
- Misses        5460     5508     +48     
- Partials       862      865      +3

Impacted Files	Coverage Δ
config/config.go	`82.58% <ø> (ø)`	⬆️
config/toml.go	`53.19% <ø> (ø)`	⬆️
consensus/reactor.go	`71.27% <0%> (-1.88%)`	⬇️
libs/db/debug_db.go	`20.63% <0%> (+4.63%)`	⬆️

ValarDragon

This seems like a good idea to me! Do we need to add libstdc++ as a dependency in the installation instructions?

melekes · 2018-09-10T10:07:02Z

Do we need to add libstdc++ as a dependency in the installation instructions?

6730374

ValarDragon · 2018-09-10T21:54:51Z

I'm kind of confused why we have syndtr/goleveldb and jmhodges/levigo, can we remove the syndtr/goleveldb dependency as well?

melekes · 2018-09-11T03:55:04Z

can we remove the syndtr/goleveldb dependency as well?

I think we still want to have a key-value DB written in Go so we can cross-compile easy. But I am less convinced now that it should be default since a) syndtr/goleveldb#226 b) it is magnitude slower than C I will try to verify this claim

ValarDragon · 2018-09-12T08:11:59Z

I'm super in favor of just using CLevelDB. I don't think its reasonable to support cross-compilation long term, if we want faster crypto speeds / or any of the newer cryptography ideas. (e.g. BLS)

ebuchman · 2018-09-12T23:19:58Z

Go-ethereum still uses syndtr/goleveldb exclusively so it could be something about the way we are configuring it? Also can you point to any popular/production products using this fork of leveldb? Is something of DataDog's using it ?

ValarDragon · 2018-09-12T23:36:42Z

We currently compact every single block. We need to significantly reduce the frequency of compactions imo.

melekes · 2018-09-13T06:45:24Z

We currently compact every single block. We need to significantly reduce the frequency of compactions imo.

yes, but this is out of scope of this PR. Here I only update outdated cleveldb library.

Also can you point to any popular/production products using this fork of leveldb? Is something of DataDog's using it ?

Datadog is using it for sure.

melekes · 2018-09-13T18:51:58Z

verify https://github.com/DataDog/leveldb/tree/master/deps/leveldb
ask DataDog guys what are they using leveldb for

melekes · 2018-09-17T08:35:12Z

DataDog/leveldb@12a0b6e

643805ecb6a4f9aaddc0652cb57ba0963162abe2c2a40b8c12aa31bbc80e645f  deps.leveldb.tar.gz
541e9d781f23b64933c8df7db7e206f047be7091c8c29e8a34a6e025a7ab960e  leveldb.v1.19.tar.gz

Files l2/include/leveldb/c.h and leveldb/deps/leveldb/include/leveldb/c.h differ
Files l2/include/leveldb/options.h and leveldb/deps/leveldb/include/leveldb/options.h differ
Files l2/port/atomic_pointer.h and leveldb/deps/leveldb/port/atomic_pointer.h differ
Files l2/table/format.cc and leveldb/deps/leveldb/table/format.cc differ
Files l2/table/table_builder.cc and leveldb/deps/leveldb/table/table_builder.cc differ
Files l2/util/options.cc and leveldb/deps/leveldb/util/options.cc differ

ask DataDog guys what are they using leveldb for

just saw there is no "Issues" tag in https://github.com/DataDog/leveldb. maybe it is a bad idea after all

Why: original fork is abandoned and not supported anymore. Changes: - LevelDB 1.19 (LevelDB and Snappy are both compiled and linked statically, so while you will not need them installed on your target machine, you should have a roughly compatible version of libstdc++.) - snappy and lz4 libs included by default

Otherwise, it's impossible to see outliers #1835 (comment)

otherwise we're benchmarking overriding single key (because hash stays the same!)

ebuchman · 2018-09-21T20:40:34Z

docs/introduction/install.md

+  make && \
+  sudo scp -r out-static/lib* out-shared/lib* /usr/local/lib/ && \
+  cd include/ && \
+  sudo scp -r leveldb /usr/local/include/ && \


can we do this all without using sudo?

think so, yeah

melekes requested review from ebuchman and xla as code owners September 5, 2018 13:10

melekes requested review from ValarDragon and liamsi September 5, 2018 13:15

ValarDragon approved these changes Sep 5, 2018

View reviewed changes

melekes requested a review from zramsay as a code owner September 10, 2018 10:56

zramsay approved these changes Sep 10, 2018

View reviewed changes

melekes mentioned this pull request Sep 12, 2018

Tendermint shows time dependent performance degredation #1835

Closed

ebuchman mentioned this pull request Sep 12, 2018

Tendermint tmlibs and common dependencies cosmos/iavl#46

Closed

melekes force-pushed the anton/cleveldb branch 2 times, most recently from 73b9ed3 to 6929ba7 Compare September 17, 2018 09:30

melekes changed the title ~~switch from jmhodges/levigo to DataDog/leveldb~~ BlockIntervalSeconds metric & DB tests cleanup & benchmark improv. Sep 17, 2018

melekes force-pushed the anton/cleveldb branch from 6929ba7 to f6b078e Compare September 17, 2018 15:24

melekes added 7 commits September 18, 2018 12:15

remove LICENSE from libs/db in favor of root license

76302c6

cleanup after tests!

747797b

update Vagrantfile to install go1.11

4841947

change consensus_block_interval_seconds metric type to gauge

788474d

Otherwise, it's impossible to see outliers #1835 (comment)

generate random txs

ff9d0cd

otherwise we're benchmarking overriding single key (because hash stays the same!)

switch back to original fork

e1bda36

melekes force-pushed the anton/cleveldb branch from 75fc256 to e1bda36 Compare September 18, 2018 08:16

melekes merged commit 64fc8f8 into develop Sep 18, 2018

melekes deleted the anton/cleveldb branch September 18, 2018 08:48

ebuchman reviewed Sep 21, 2018

View reviewed changes

Conversation

melekes commented Sep 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Sep 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ValarDragon left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

melekes commented Sep 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ValarDragon commented Sep 10, 2018

Uh oh!

melekes commented Sep 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ValarDragon commented Sep 12, 2018

Uh oh!

ebuchman commented Sep 12, 2018

Uh oh!

ValarDragon commented Sep 12, 2018

Uh oh!

melekes commented Sep 13, 2018

Uh oh!

melekes commented Sep 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

melekes commented Sep 17, 2018

Uh oh!

ebuchman Sep 21, 2018

Choose a reason for hiding this comment

Uh oh!

melekes Sep 23, 2018

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

melekes commented Sep 5, 2018 •

edited

Loading

codecov-io commented Sep 5, 2018 •

edited

Loading

ValarDragon left a comment •

edited

Loading

melekes commented Sep 10, 2018 •

edited

Loading

melekes commented Sep 11, 2018 •

edited

Loading

melekes commented Sep 13, 2018 •

edited

Loading