Add Task DB implementation using a local BoltDB.

Add Task DB implementation using a local BoltDB.

This is based on https://skia.googlesource.com/buildbot/+/b3e65910018762c87e786a3b7bd37a601cec24d0/go/buildbot/local_db.go 

Moved db/db_test.go to db/testutil.go to overcome circular dependency issue in local_db_test.go. Moved tests to memory_test.go.

Many TODOs added for missing functionality.

BUG=skia:5626
Committed: https://skia.googlesource.com/buildbot/+/4ca08f07c538b7dd11dbb9ee3a46139d1c7b3203

[dogben, 2016-08-15 17:11:51]

The CQ bit was checked by benjaminwagner@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-15 17:11:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-15 17:17:22]

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

[commit-bot: I haz the power, 2016-08-15 17:17:23]

Dry run: This issue passed the CQ dry run.

[dogben, 2016-08-15 17:43:00]

benjaminwagner@google.com changed reviewers:
+ borenet@google.com

[dogben, 2016-08-15 21:50:45]

Description was changed from

==========
Add Task DB implementation using a local BoltDB.

Moved db/db_test.go to db/testutil.go to overcome circular dependency issue in
local_db_test.go. Moved tests to memory_test.go.

Many TODOs added for missing functionality.

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

to

==========
Add Task DB implementation using a local BoltDB.

This is based on
https://skia.googlesource.com/buildbot/+/b3e65910018762c87e786a3b7bd37a601cec...


Moved db/db_test.go to db/testutil.go to overcome circular dependency issue in
local_db_test.go. Moved tests to memory_test.go.

Many TODOs added for missing functionality.

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

[borenet, 2016-08-16 19:49:54]

Just a couple of comments.

https://codereview.chromium.org/2246933002/diff/1/build_scheduler/go/db/local...
File build_scheduler/go/db/local_db/local_db.go (right):

https://codereview.chromium.org/2246933002/diff/1/build_scheduler/go/db/local...
build_scheduler/go/db/local_db/local_db.go:48: // single hexidecimal digit.)
Why not just have this function zero-pad the sequence number, which IMO would be
easier to read?  That'd also give us a fixed-width ID, which may be convenient.

https://codereview.chromium.org/2246933002/diff/1/build_scheduler/go/db/local...
build_scheduler/go/db/local_db/local_db.go:200: d.txActive = map[int64]string{}
nit: Should we also set the counter to zero here?

[dogben, 2016-08-16 22:22:36]

The CQ bit was checked by benjaminwagner@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-16 22:22:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-16 23:12:36]

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

[commit-bot: I haz the power, 2016-08-16 23:12:37]

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

[dogben, 2016-08-17 13:29:15]

The CQ bit was checked by benjaminwagner@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-17 13:29:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dogben, 2016-08-17 13:29:44]

https://codereview.chromium.org/2246933002/diff/1/build_scheduler/go/db/local...
File build_scheduler/go/db/local_db/local_db.go (right):

https://codereview.chromium.org/2246933002/diff/1/build_scheduler/go/db/local...
build_scheduler/go/db/local_db/local_db.go:48: // single hexidecimal digit.)
On 2016/08/16 19:49:54, borenet wrote:
> Why not just have this function zero-pad the sequence number, which IMO would
be
> easier to read?  That'd also give us a fixed-width ID, which may be
convenient.

You're right. I thought it was overkill to have the full 16 bytes on the Id, but
it makes things much simpler.

https://codereview.chromium.org/2246933002/diff/1/build_scheduler/go/db/local...
build_scheduler/go/db/local_db/local_db.go:200: d.txActive = map[int64]string{}
On 2016/08/16 19:49:54, borenet wrote:
> nit: Should we also set the counter to zero here?

I added a check that there are no active transactions, so txCount should also be
zero. Added a TODO to delete the metric.

[commit-bot: I haz the power, 2016-08-17 13:34:48]

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

[commit-bot: I haz the power, 2016-08-17 13:34:49]

Dry run: This issue passed the CQ dry run.

[dogben, 2016-08-17 13:35:43]

Sorry, it seems I have yet another bad merge. Will fix it momentarily...

[dogben, 2016-08-17 13:38:52]

On 2016/08/17 13:35:43, Ben Wagner wrote:
> Sorry, it seems I have yet another bad merge. Will fix it momentarily...

Nvm, I was confused by the diff-from-previous. Good to go.

[borenet, 2016-08-17 13:51:13]

LGTM

https://codereview.chromium.org/2246933002/diff/40001/build_scheduler/go/db/l...
File build_scheduler/go/db/local_db/local_db.go (right):

https://codereview.chromium.org/2246933002/diff/40001/build_scheduler/go/db/l...
build_scheduler/go/db/local_db/local_db.go:199: // TODO(benjaminwagner): Make
this work.
What's not working?

[dogben, 2016-08-17 13:52:55]

The CQ bit was checked by benjaminwagner@google.com

[commit-bot: I haz the power, 2016-08-17 13:52:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-17 13:53:17]

Description was changed from

==========
Add Task DB implementation using a local BoltDB.

This is based on
https://skia.googlesource.com/buildbot/+/b3e65910018762c87e786a3b7bd37a601cec...


Moved db/db_test.go to db/testutil.go to overcome circular dependency issue in
local_db_test.go. Moved tests to memory_test.go.

Many TODOs added for missing functionality.

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

to

==========
Add Task DB implementation using a local BoltDB.

This is based on
https://skia.googlesource.com/buildbot/+/b3e65910018762c87e786a3b7bd37a601cec...


Moved db/db_test.go to db/testutil.go to overcome circular dependency issue in
local_db_test.go. Moved tests to memory_test.go.

Many TODOs added for missing functionality.

BUG=skia:5626

Committed:
https://skia.googlesource.com/buildbot/+/4ca08f07c538b7dd11dbb9ee3a46139d1c7b...
==========

[commit-bot: I haz the power, 2016-08-17 13:53:18]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/buildbot/+/4ca08f07c538b7dd11dbb9ee3a46139d1c7b...

[dogben, 2016-08-17 13:57:58]

https://codereview.chromium.org/2246933002/diff/40001/build_scheduler/go/db/l...
File build_scheduler/go/db/local_db/local_db.go (right):

https://codereview.chromium.org/2246933002/diff/40001/build_scheduler/go/db/l...
build_scheduler/go/db/local_db/local_db.go:199: // TODO(benjaminwagner): Make
this work.
On 2016/08/17 13:51:13, borenet wrote:
> What's not working?

Actually, it would be great if you can take a look at this. I'm a bit stumped as
to why it's not working. I added a Delete method to Counter that just calls
c.m.Delete(), but I got "Unable to delete unknown metric:
counter_6e5218a88b3b4da8208da44a3ada65ae". (Counter.Delete also needs to delete
from c.m.client.counters, but I didn't get that far.)

[borenet, 2016-08-19 13:34:00]

On 2016/08/17 13:57:58, Ben Wagner wrote:
>
https://codereview.chromium.org/2246933002/diff/40001/build_scheduler/go/db/l...
> File build_scheduler/go/db/local_db/local_db.go (right):
> 
>
https://codereview.chromium.org/2246933002/diff/40001/build_scheduler/go/db/l...
> build_scheduler/go/db/local_db/local_db.go:199: // TODO(benjaminwagner): Make
> this work.
> On 2016/08/17 13:51:13, borenet wrote:
> > What's not working?
> 
> Actually, it would be great if you can take a look at this. I'm a bit stumped
as
> to why it's not working. I added a Delete method to Counter that just calls
> c.m.Delete(), but I got "Unable to delete unknown metric:
> counter_6e5218a88b3b4da8208da44a3ada65ae". (Counter.Delete also needs to
delete
> from c.m.client.counters, but I didn't get that far.)

Okay, I just created a simple example but was unable to reproduce this failure:

///////////////// counter.go

// Delete removes the counter from metrics.
func (c *Counter) Delete() error {
        c.mtx.Lock()
        defer c.mtx.Unlock()
        c.m.client.countersMtx.Lock()
        defer c.m.client.countersMtx.Unlock()
        delete(c.m.client.counters, c.m.key)
        return c.m.Delete()
}


///////////////// delete_counter.go

package main

import (
        "github.com/skia-dev/glog"
        "go.skia.org/infra/go/common"
        "go.skia.org/infra/go/metrics2"
)

func main() {
        common.Init()
        defer common.LogPanic()

        c := metrics2.GetCounter("my-counter", map[string]string{"my-tag":
"my-val"})
        if err := c.Delete(); err != nil {
                glog.Fatal(err)
        }
}