Create gRPC client and server, traceservice, that stores trace data in a BoltDB backend.

A new datastore for traces (CL 1 of 4).

Create gRPC client and server, traceservice, that stores trace data in a BoltDB backend.

The important bit that there will only be two users of traceservice, the version of tracedb.DB that's backed by traceservice (coming in another CL), and the traceservice itself (also coming in another CL)

BUG=skia:

Committed: https://skia.googlesource.com/buildbot/+/a8c24924c50ba9f3a58075c0ac893033dc85333e

[jcgregorio, 2015-10-16 19:39:51]

jcgregorio@google.com changed reviewers:
+ mtklein@google.com, stephana@google.com

[stephana, 2015-10-19 15:03:49]

https://codereview.chromium.org/1411663004/diff/20001/go/tracedb/DESIGN.md
File go/tracedb/DESIGN.md (right):

https://codereview.chromium.org/1411663004/diff/20001/go/tracedb/DESIGN.md#ne...
go/tracedb/DESIGN.md:123: Close() error
There is no way to enumerate the CommitIDs currently in the data store. As a
client I want to be able to traverse the data store, if for no other reason that
ensure consistency.  

A simple solution would be to expose the min/max and number of entries in the
store, that way I could easily devise a strategy to traverse the entire store.

https://codereview.chromium.org/1411663004/diff/20001/go/tracedb/DESIGN.md#ne...
go/tracedb/DESIGN.md:134: }
typo: Rietveld 

I don't see a simple way to enumerate recent Rietveld issues without maintaining
a secondary data store with the ingested trybot results/Rietveld issues.

[jcgregorio, 2015-10-19 15:11:44]

https://codereview.chromium.org/1411663004/diff/20001/go/tracedb/DESIGN.md
File go/tracedb/DESIGN.md (right):

https://codereview.chromium.org/1411663004/diff/20001/go/tracedb/DESIGN.md#ne...
go/tracedb/DESIGN.md:123: Close() error
On 2015/10/19 at 15:03:49, stephana wrote:
> There is no way to enumerate the CommitIDs currently in the data store. As a
client I want to be able to traverse the data store, if for no other reason that
ensure consistency.  

To do that simply call:

  List(time.Time{}, time.Now())


> 
> A simple solution would be to expose the min/max and number of entries in the
store, that way I could easily devise a strategy to traverse the entire store.

https://codereview.chromium.org/1411663004/diff/20001/go/tracedb/DESIGN.md#ne...
go/tracedb/DESIGN.md:134: }
On 2015/10/19 at 15:03:49, stephana wrote:
> typo: Rietveld 
> 
> I don't see a simple way to enumerate recent Rietveld issues without
maintaining a secondary data store with the ingested trybot results/Rietveld
issues.

Use List() with a beginning and ending time that you are interested in and then
filter the resulting CommitIDs that have a Source that is a Rietveld ID.
Probably the best Source identifier to use for a trybot would be the Rietveld
URL.

[stephana, 2015-10-19 15:22:40]

https://codereview.chromium.org/1411663004/diff/20001/go/tracedb/DESIGN.md
File go/tracedb/DESIGN.md (right):

https://codereview.chromium.org/1411663004/diff/20001/go/tracedb/DESIGN.md#ne...
go/tracedb/DESIGN.md:134: }
On 2015/10/19 15:11:44, jcgregorio wrote:
> On 2015/10/19 at 15:03:49, stephana wrote:
> > typo: Rietveld 
> > 
> > I don't see a simple way to enumerate recent Rietveld issues without
> maintaining a secondary data store with the ingested trybot results/Rietveld
> issues.
> 
> Use List() with a beginning and ending time that you are interested in and
then
> filter the resulting CommitIDs that have a Source that is a Rietveld ID.
> Probably the best Source identifier to use for a trybot would be the Rietveld
> URL.

That means I have to load the equivalent of a current tile (ballpark) to get to
a single trybot result. Currently on non-SSD disks this takes 30+ seconds. I
understand you want to keep the interface simple, but there needs to be some
filtering/searching at the storage level, i.e. like the prefix scan in boltdb.

[jcgregorio, 2015-10-19 15:29:01]

On 2015/10/19 at 15:22:40, stephana wrote:
> https://codereview.chromium.org/1411663004/diff/20001/go/tracedb/DESIGN.md
> File go/tracedb/DESIGN.md (right):
> 
>
https://codereview.chromium.org/1411663004/diff/20001/go/tracedb/DESIGN.md#ne...
> go/tracedb/DESIGN.md:134: }
> On 2015/10/19 15:11:44, jcgregorio wrote:
> > On 2015/10/19 at 15:03:49, stephana wrote:
> > > typo: Rietveld 
> > > 
> > > I don't see a simple way to enumerate recent Rietveld issues without
> > maintaining a secondary data store with the ingested trybot results/Rietveld
> > issues.
> > 
> > Use List() with a beginning and ending time that you are interested in and
then
> > filter the resulting CommitIDs that have a Source that is a Rietveld ID.
> > Probably the best Source identifier to use for a trybot would be the
Rietveld
> > URL.
> 
> That means I have to load the equivalent of a current tile (ballpark) to get
to a single trybot result. Currently on non-SSD disks this takes 30+ seconds. I
understand you want to keep the interface simple, but there needs to be some
filtering/searching at the storage level, i.e. like the prefix scan in boltdb.

That's not how the data is stored in Bolt, nor is that how it's going to be
stored in BigTable. For example, in the gRPC BoltDB implementation, in this CL,
the List method is implemented as a prefix scan on bucket names. The bucket
names are then converted to CommitIDs, the actual row data is never loaded. The
BigTable implementation will work similarly.

[stephana, 2015-10-19 20:00:10]

On 2015/10/19 15:29:01, jcgregorio wrote:
> On 2015/10/19 at 15:22:40, stephana wrote:
> > https://codereview.chromium.org/1411663004/diff/20001/go/tracedb/DESIGN.md
> > File go/tracedb/DESIGN.md (right):
> > 
> >
>
https://codereview.chromium.org/1411663004/diff/20001/go/tracedb/DESIGN.md#ne...
> > go/tracedb/DESIGN.md:134: }
> > On 2015/10/19 15:11:44, jcgregorio wrote:
> > > On 2015/10/19 at 15:03:49, stephana wrote:
> > > > typo: Rietveld 
> > > > 
> > > > I don't see a simple way to enumerate recent Rietveld issues without
> > > maintaining a secondary data store with the ingested trybot
results/Rietveld
> > > issues.
> > > 
> > > Use List() with a beginning and ending time that you are interested in and
> then
> > > filter the resulting CommitIDs that have a Source that is a Rietveld ID.
> > > Probably the best Source identifier to use for a trybot would be the
> Rietveld
> > > URL.
> > 
> > That means I have to load the equivalent of a current tile (ballpark) to get
> to a single trybot result. Currently on non-SSD disks this takes 30+ seconds.
I
> understand you want to keep the interface simple, but there needs to be some
> filtering/searching at the storage level, i.e. like the prefix scan in boltdb.
> 
> That's not how the data is stored in Bolt, nor is that how it's going to be
> stored in BigTable. For example, in the gRPC BoltDB implementation, in this
CL,
> the List method is implemented as a prefix scan on bucket names. The bucket
> names are then converted to CommitIDs, the actual row data is never loaded.
The
> BigTable implementation will work similarly.

I don't see a scan over bucket names, but it looks like it scans the contents of
the commits bucket, but the effect is the same. 

Can we add a flag or convention to the CommitID to know what the source is? i.e.
have a 'g:' prefix for git hashes and an r: prefix for Rietfeld issues.
Otherwise I have no way of telling the difference other than looking at the
length off the id. Which seems very brittle. The alternative would be to add an
explicit flag for the source type.

[stephana, 2015-10-19 20:00:19]

https://codereview.chromium.org/1411663004/diff/20001/go/tracedb/DESIGN.md
File go/tracedb/DESIGN.md (right):

https://codereview.chromium.org/1411663004/diff/20001/go/tracedb/DESIGN.md#ne...
go/tracedb/DESIGN.md:220: map[uint64][]byte, where the uint64 is the trace64id.
Shouldn't this be the '!' delimited concatenation of the CommitID described
above ?

https://codereview.chromium.org/1411663004/diff/20001/traceservice/proto/impl.go
File traceservice/proto/impl.go (right):

https://codereview.chromium.org/1411663004/diff/20001/traceservice/proto/impl...
traceservice/proto/impl.go:32: func BytesFromInt64(u uint64) []byte {
nit: Should this be named something like BytesFromUInt64(u uint64) 
same for other functions below. 

If thes functions are only used in this package they should be private.
Otherwise they should be factored out into the shared library.

https://codereview.chromium.org/1411663004/diff/20001/traceservice/proto/impl...
traceservice/proto/impl.go:49: func CommitIDToBytes(c *CommitID) []byte {
There is the (probably unlikely) possibility that a part of the commitID
contains a '!'. If that was the case we would have the problem that we write
something to the database, but cannot read it back.

[jcgregorio, 2015-10-19 20:30:11]

https://codereview.chromium.org/1411663004/diff/20001/go/tracedb/DESIGN.md
File go/tracedb/DESIGN.md (right):

https://codereview.chromium.org/1411663004/diff/20001/go/tracedb/DESIGN.md#ne...
go/tracedb/DESIGN.md:134: }
Fixed Typo.

https://codereview.chromium.org/1411663004/diff/20001/go/tracedb/DESIGN.md#ne...
go/tracedb/DESIGN.md:220: map[uint64][]byte, where the uint64 is the trace64id.
Fixed.

On 2015/10/19 at 20:00:19, stephana wrote:
> Shouldn't this be the '!' delimited concatenation of the CommitID described
above ?

https://codereview.chromium.org/1411663004/diff/20001/traceservice/proto/impl.go
File traceservice/proto/impl.go (right):

https://codereview.chromium.org/1411663004/diff/20001/traceservice/proto/impl...
traceservice/proto/impl.go:32: func BytesFromInt64(u uint64) []byte {
Changed to Uint and made private.

On 2015/10/19 at 20:00:19, stephana wrote:
> nit: Should this be named something like BytesFromUInt64(u uint64) 
> same for other functions below. 
> 
> If thes functions are only used in this package they should be private.
Otherwise they should be factored out into the shared library.

https://codereview.chromium.org/1411663004/diff/20001/traceservice/proto/impl...
traceservice/proto/impl.go:49: func CommitIDToBytes(c *CommitID) []byte {
Good catch, added a check for ! and return an error if it appears in either the
Id or the Source.

On 2015/10/19 at 20:00:19, stephana wrote:
> There is the (probably unlikely) possibility that a part of the commitID
contains a '!'. If that was the case we would have the problem that we write
something to the database, but cannot read it back.

[jcgregorio, 2015-10-19 20:58:12]

Description was changed from

==========
A new datastore for traces (CL 1 of 4).

Create gRPC client and server, traceservice, that stores trace data in a BoltDB
backend.

The important bit that there will only be two users of traceservice, the version
of tracedb.DB that's backed by traceservice (coming in another CL), and the
traceservice itself (also coming in another CL)

There will also a BigTable backed implementation of tracedb.DB (in another CL).

BUG=skia:
==========

to

==========
A new datastore for traces (CL 1 of 4).

Create gRPC client and server, traceservice, that stores trace data in a BoltDB
backend.

The important bit that there will only be two users of traceservice, the version
of tracedb.DB that's backed by traceservice (coming in another CL), and the
traceservice itself (also coming in another CL)

BUG=skia:
==========

[stephana, 2015-10-20 14:42:32]

lgtm with one comment

https://codereview.chromium.org/1411663004/diff/20001/go/tracedb/DESIGN.md
File go/tracedb/DESIGN.md (right):

https://codereview.chromium.org/1411663004/diff/20001/go/tracedb/DESIGN.md#ne...
go/tracedb/DESIGN.md:1: tracedb
Could go/traceservice/ be merged into go/tracedb/ or vice versa.

[jcgregorio, 2015-10-20 14:51:44]

On 2015/10/20 at 14:42:32, stephana wrote:
> lgtm with one comment
> 
> https://codereview.chromium.org/1411663004/diff/20001/go/tracedb/DESIGN.md
> File go/tracedb/DESIGN.md (right):
> 
>
https://codereview.chromium.org/1411663004/diff/20001/go/tracedb/DESIGN.md#ne...
> go/tracedb/DESIGN.md:1: tracedb
> Could go/traceservice/ be merged into go/tracedb/ or vice versa.

It made sense to have them separate when there was going to be a BoltDB and a
BigTable backend. Since we've abandoned that I will merge all of this under a
single /tracedb directory.

[jcgregorio, 2015-10-20 15:09:50]

The CQ bit was checked by jcgregorio@google.com

[jcgregorio, 2015-10-20 15:09:50]

The patchset sent to the CQ was uploaded after l-g-t-m from stephana@google.com
Link to the patchset: https://codereview.chromium.org/1411663004/#ps80001
(title: "rearrange files")

[commit-bot: I haz the power, 2015-10-20 15:10:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1411663004/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1411663004/80001

[commit-bot: I haz the power, 2015-10-20 15:12:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-20 15:12:20]

Try jobs failed on following builders:
  Infra-PerCommit-Trybot on client.skia.fyi (JOB_FAILED,
http://build.chromium.org/p/client.skia.fyi/builders/Infra-PerCommit-Trybot/b...)

[jcgregorio, 2015-10-20 15:12:45]

The CQ bit was checked by jcgregorio@google.com

[jcgregorio, 2015-10-20 15:12:46]

The patchset sent to the CQ was uploaded after l-g-t-m from stephana@google.com
Link to the patchset: https://codereview.chromium.org/1411663004/#ps100001
(title: "fix vet")

[commit-bot: I haz the power, 2015-10-20 15:12:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1411663004/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1411663004/100001

[commit-bot: I haz the power, 2015-10-20 15:14:18]

Committed patchset #6 (id:100001) as
https://skia.googlesource.com/buildbot/+/a8c24924c50ba9f3a58075c0ac893033dc85...