Skip to content
This repository was archived by the owner on Oct 11, 2024. It is now read-only.

Comments

Extract log tracking into separate package #1317

Merged
gdbelvin merged 18 commits intogoogle:masterfrom
gdbelvin:tracker
Jul 18, 2019
Merged

Extract log tracking into separate package #1317
gdbelvin merged 18 commits intogoogle:masterfrom
gdbelvin:tracker

Conversation

@gdbelvin
Copy link
Contributor

@gdbelvin gdbelvin commented Jul 11, 2019

This PR isolates root tracking inside a new tracker package.
By putting all root tracking behind an interface, we can implement alternate, multi-threaded root trackers. #1311

@gdbelvin gdbelvin marked this pull request as ready for review July 16, 2019 17:38
@gdbelvin gdbelvin requested a review from a team as a code owner July 16, 2019 17:38
@codecov
Copy link

codecov bot commented Jul 16, 2019

Codecov Report

Merging #1317 into master will increase coverage by 0.2%.
The diff coverage is 37.27%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #1317     +/-   ##
=========================================
+ Coverage   30.09%   30.29%   +0.2%     
=========================================
  Files          47       48      +1     
  Lines        3825     3869     +44     
=========================================
+ Hits         1151     1172     +21     
- Misses       2496     2516     +20     
- Partials      178      181      +3
Impacted Files Coverage Δ
core/monitor/monitor.go 19.71% <ø> (ø) ⬆️
core/client/client.go 28.94% <ø> (-1.25%) ⬇️
core/integration/client_tests.go 0% <0%> (ø) ⬆️
core/client/batch_get_and_verify.go 0% <0%> (ø) ⬆️
core/client/get_and_verify.go 23.45% <21.42%> (-0.14%) ⬇️
core/client/verifier/pairs.go 21.05% <50%> (ø) ⬆️
core/client/tracker/tracker.go 66.66% <66.66%> (ø)
core/client/verifier/verifier.go 56% <75%> (+0.28%) ⬆️

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 b4dc4bd...3f04785. Read the comment docs.

@gdbelvin gdbelvin requested a review from pav-kv July 17, 2019 10:03

cli, logTracker := NewClientWithTracker(t, env)
logTracker.SetUpdatePredicate(func(_, newRoot types.LogRootV1) bool {
return newRoot.TreeSize%5 == 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to test this predicate, and not the original one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question.
a) The original predicate is tested in the tracker's unit tests.
b) The purpose of this is to produce more interesting consistency proofs in the test vectors. If we updated the log root every time, the consistency proofs would always be empty.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a comment explaining this?

@gdbelvin gdbelvin requested a review from pav-kv July 18, 2019 10:55
Copy link
Contributor

@pav-kv pav-kv left a comment

Choose a reason for hiding this comment

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

LGTM % nits.


cli, logTracker := NewClientWithTracker(t, env)
logTracker.SetUpdatePredicate(func(_, newRoot types.LogRootV1) bool {
return newRoot.TreeSize%5 == 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a comment explaining this?

@gdbelvin gdbelvin merged commit de5b12d into google:master Jul 18, 2019
@gdbelvin gdbelvin deleted the tracker branch July 18, 2019 13:22
gdbelvin added a commit to gdbelvin/keytransparency that referenced this pull request Jul 18, 2019
* master:
  Extract log tracking into separate package  (google#1317)
gdbelvin added a commit to gdbelvin/keytransparency that referenced this pull request Jul 18, 2019
* master:
  Cache go mod download (google#1325)
  Extract log tracking into separate package  (google#1317)
gdbelvin added a commit to gdbelvin/keytransparency that referenced this pull request Jul 18, 2019
* master:
  Cache go mod download (google#1325)
  Extract log tracking into separate package  (google#1317)
@gdbelvin gdbelvin mentioned this pull request Aug 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants