Improve memory implementation of gae to provide the full RawDatastore interface.

Improve memory implementation of gae to provide the full RawDatastore interface.

Still no query support yet, working on that next, but this enables *Multi apis.
Also do a few other code cleanups.

R=dnj@chromium.org, estaab@chromium.org, martiniss@chromium.org, vadimsh@chromium.org
BUG=

Committed: https://chromium.googlesource.com/infra/infra/+/6a76f7841de5619824a3e743f2f8571e53549688

[iannucci, 2015-07-17 01:14:56]

this one's now ready to rock

[Vadim Sh., 2015-07-17 01:48:30]

lgtm, though I did not really understand what I've just reviewed (logic)...

https://codereview.chromium.org/1242043005/diff/80001/go/src/infra/gae/libs/g...
File go/src/infra/gae/libs/gae/memory/raw_datstore_data.go (right):

https://codereview.chromium.org/1242043005/diff/80001/go/src/infra/gae/libs/g...
go/src/infra/gae/libs/gae/memory/raw_datstore_data.go:90: func (d
*dataStoreData) entsKeyLocked(key gae.DSKey) (*memCollection, gae.DSKey) {
do you have tests that spawn multiple goroutines and hammer the DB concurrently?
It'll be great to run them under race detector.

https://codereview.chromium.org/1242043005/diff/80001/go/src/infra/gae/libs/g...
go/src/infra/gae/libs/gae/memory/raw_datstore_data.go:239: if err != nil {
nit: if err := ...; err != nil {

https://codereview.chromium.org/1242043005/diff/80001/go/src/infra/gae/libs/g...
go/src/infra/gae/libs/gae/memory/raw_datstore_data.go:436: _, k =
td.parent.entsKeyLocked(k)
why not:
td.parent.Lock()
_, k = td.parent.entsKeyLocked(k)
td.parent.Unlock()

?

anonymous functions in loops, executed right away, eery resemble javascript...

[iannucci, 2015-07-17 02:43:25]

On 2015/07/17 01:48:30, Vadim Sh. wrote:
> lgtm, though I did not really understand what I've just reviewed (logic)...
> 
>
https://codereview.chromium.org/1242043005/diff/80001/go/src/infra/gae/libs/g...
> File go/src/infra/gae/libs/gae/memory/raw_datstore_data.go (right):
> 
>
https://codereview.chromium.org/1242043005/diff/80001/go/src/infra/gae/libs/g...
> go/src/infra/gae/libs/gae/memory/raw_datstore_data.go:90: func (d
> *dataStoreData) entsKeyLocked(key gae.DSKey) (*memCollection, gae.DSKey) {
> do you have tests that spawn multiple goroutines and hammer the DB
concurrently?
> It'll be great to run them under race detector.

Yeah I need to. Was waiting until I had query implemented (next CL after this
one, wip).

> 
>
https://codereview.chromium.org/1242043005/diff/80001/go/src/infra/gae/libs/g...
> go/src/infra/gae/libs/gae/memory/raw_datstore_data.go:239: if err != nil {
> nit: if err := ...; err != nil {
> 
>
https://codereview.chromium.org/1242043005/diff/80001/go/src/infra/gae/libs/g...
> go/src/infra/gae/libs/gae/memory/raw_datstore_data.go:436: _, k =
> td.parent.entsKeyLocked(k)
> why not:
> td.parent.Lock()
> _, k = td.parent.entsKeyLocked(k)
> td.parent.Unlock()
> 
> ?
> 
> anonymous functions in loops, executed right away, eery resemble javascript...

panic maybe? I dunno... I don't like to do the `lock; fn(); unlock` pattern...
it's OK if fn() is obviously trivial (like reading from a map or an array or
something). Also keep in mind that even if entsKeyLocked doesn't panic today, it
could panic in the future, at which point this turns into a deadlock.

That said... this is the testing implementation after all :)

[iannucci, 2015-07-17 02:47:28]

On 2015/07/17 01:48:30, Vadim Sh. wrote:
> lgtm, though I did not really understand what I've just reviewed (logic)...

If you want, I can walk through it a bit? It was mostly just adding the *Multi
methods which were missing before, and removing errors from some functions where
it's impossible for them to have errors (memory corruption notwithstanding,
hence the panics).

> 
>
https://codereview.chromium.org/1242043005/diff/80001/go/src/infra/gae/libs/g...
> File go/src/infra/gae/libs/gae/memory/raw_datstore_data.go (right):
> 
>
https://codereview.chromium.org/1242043005/diff/80001/go/src/infra/gae/libs/g...
> go/src/infra/gae/libs/gae/memory/raw_datstore_data.go:90: func (d
> *dataStoreData) entsKeyLocked(key gae.DSKey) (*memCollection, gae.DSKey) {
> do you have tests that spawn multiple goroutines and hammer the DB
concurrently?
> It'll be great to run them under race detector.
> 
>
https://codereview.chromium.org/1242043005/diff/80001/go/src/infra/gae/libs/g...
> go/src/infra/gae/libs/gae/memory/raw_datstore_data.go:239: if err != nil {
> nit: if err := ...; err != nil {
> 
>
https://codereview.chromium.org/1242043005/diff/80001/go/src/infra/gae/libs/g...
> go/src/infra/gae/libs/gae/memory/raw_datstore_data.go:436: _, k =
> td.parent.entsKeyLocked(k)
> why not:
> td.parent.Lock()
> _, k = td.parent.entsKeyLocked(k)
> td.parent.Unlock()
> 
> ?
> 
> anonymous functions in loops, executed right away, eery resemble javascript...

[iannucci, 2015-07-17 03:09:25]

The CQ bit was checked by iannucci@chromium.org

[commit-bot: I haz the power, 2015-07-17 03:09:39]

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

[commit-bot: I haz the power, 2015-07-17 03:13:24]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/infra/infra/+/6a76f7841de5619824a3e743f2f85...