add elastic search store

[jrmdayn]

inspired by couchstore impl
tests (store + tmpop) run in ~30sec
I am using this elastic client in go: https://github.com/olivere/elastic

fix #349

---

This change is 

[codecov]

Codecov Report

Merging #357 into master will increase coverage by 1.88%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master    #357      +/-   ##
=========================================
+ Coverage   76.41%   78.3%   +1.88%     
=========================================
  Files          71      71              
  Lines        4686    5204     +518     
=========================================
+ Hits         3581    4075     +494     
- Misses        835     853      +18     
- Partials      270     276       +6

Impacted Files	Coverage Δ
validator/pki.go	90.38% <0%> (-4.74%)	⬇️
validator/configloader.go	90.62% <0%> (-3.41%)	⬇️
validator/schema.go	78.57% <0%> (ø)	⬆️
utils/docker.go	0% <0%> (ø)	⬆️
validator/baseconfig.go	100% <0%> (ø)	⬆️
bufferedbatch/batch.go	96% <0%> (+0.44%)	⬆️
tmstore/tmstore.go	71.73% <0%> (+4.22%)	⬆️
cs/cs.go	89.25% <0%> (+4.52%)	⬆️
batchfossilizer/batchfossilizer.go	82.82% <0%> (+5.83%)	⬆️
rethinkstore/rethinkstore.go	92.45% <0%> (+8%)	⬆️
... and 1 more

---

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 4d62d08...378fd8e. Read the comment docs.

[such]

Good job! Sorry for the nitpicking...

---

Reviewed 8 of 8 files at r1.
Review status: 7 of 8 files reviewed at latest revision, 6 unresolved discussions.

---

elasticsearchstore/api.go, line 37 at r1 (raw file):

// Evidences is a wrapper around cs.Evidences for json ElasticSearch serialization compliance
// Elastic Search does not allow indexing of arrays directly..

nit: only 1 dot

---

elasticsearchstore/api.go, line 43 at r1 (raw file):

// Value is a wrapper struct for the value of the keyvalue store part
// Elastic only accepts json structured objects

nit: we're trying to end comments with a dot.

---

elasticsearchstore/api.go, line 61 at r1 (raw file):

		if !createIndex.Acknowledged {
			// Not acknowledged
			return fmt.Errorf("Error creating %s index", indexName)

nit: errors should be in lower case.

---

elasticsearchstore/elasticsearchstore.go, line 68 at r1 (raw file):
We're trying to follow the best pratice for context even if that feels a bit inconfortable:

Do not store Contexts inside a struct type; instead, pass a Context explicitly to each function that needs it. The Context should be the first parameter, typically named ctx:

https://golang.org/pkg/context/

---

elasticsearchstore/elasticsearchstore.go, line 205 at r1 (raw file):

}

/********** github.com/stratumn/go-indigocore/store.KeyValueStore implementation **********/

I'm not sure this was really needed but I guess it can be useful at some point. It doesn't seem to bloat the code too much.

---

elasticsearchstore/elasticsearchstore_test.go, line 77 at r1 (raw file):

		fmt.Printf(err.Error())
		os.Exit(1)
	}

nit: we could probably factorize this code at some point.

---

Comments from Reviewable

[such]

Review status: 7 of 8 files reviewed at latest revision, 7 unresolved discussions, some commit checks failed.

---

elasticsearchstore/elasticsearchstore_test.go, line 95 at r2 (raw file):

func TestElasticSearchStore(t *testing.T) {
	if *integration {

do we really want to consider this test as integration?

---

Comments from Reviewable

[such]

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 7 unresolved discussions, some commit checks failed.

---

Comments from Reviewable

[jrmdayn]

Review status: all files reviewed at latest revision, 7 unresolved discussions, some commit checks failed.

---

elasticsearchstore/api.go, line 37 at r1 (raw file):

Previously, such (Adrien Montfort) wrote…

nit: only 1 dot

ok

---

elasticsearchstore/api.go, line 43 at r1 (raw file):

Previously, such (Adrien Montfort) wrote…

nit: we're trying to end comments with a dot.

gotcha

---

elasticsearchstore/api.go, line 61 at r1 (raw file):

Previously, such (Adrien Montfort) wrote…

nit: errors should be in lower case.

gotcha

---

elasticsearchstore/elasticsearchstore.go, line 68 at r1 (raw file):

Previously, such (Adrien Montfort) wrote…

We're trying to follow the best pratice for context even if that feels a bit inconfortable:

Do not store Contexts inside a struct type; instead, pass a Context explicitly to each function that needs it. The Context should be the first parameter, typically named ctx:

https://golang.org/pkg/context/

I see, I'll change that

---

elasticsearchstore/elasticsearchstore.go, line 205 at r1 (raw file):

Previously, such (Adrien Montfort) wrote…

I'm not sure this was really needed but I guess it can be useful at some point. It doesn't seem to bloat the code too much.

You mean that ESStore does not need to be a KeyValueStore? When do we use KVStore? Tests would fail if not implemented I suppose?

---

elasticsearchstore/elasticsearchstore_test.go, line 77 at r1 (raw file):

Previously, such (Adrien Montfort) wrote…

nit: we could probably factorize this code at some point.

agreed

---

elasticsearchstore/elasticsearchstore_test.go, line 95 at r2 (raw file):

Previously, such (Adrien Montfort) wrote…

do we really want to consider this test as integration?

I had all sorts of problems on travis and semaphore CI:
https://semaphoreci.com/stratumn/go-indigocore/branches/jd-elasticsearchstore/builds/1
https://travis-ci.org/stratumn/go-indigocore/builds/349873828?utm_source=github_status&utm_medium=notification

the travis error means that it takes too long to even start elasticsearch container..

---

Comments from Reviewable

[bejito]

elasticsearchstore/api.go, line 265 at r2 (raw file):

		}
	}
	return filter.PaginateStrings(res), nil

Shouldn't we directly paginate the call to ES ?

---

Comments from Reviewable

[such]

Review status: all files reviewed at latest revision, 8 unresolved discussions, some commit checks failed.

---

elasticsearchstore/elasticsearchstore.go, line 205 at r1 (raw file):

Previously, jeremie-stratumn (Jeremie Dayan) wrote…

You mean that ESStore does not need to be a KeyValueStore? When do we use KVStore? Tests would fail if not implemented I suppose?

No you're right. Right now I don't think there's a way to test only the LinkStore. But the KVStore is only used in tmpop and we have a generic implementation.

---

elasticsearchstore/elasticsearchstore_test.go, line 95 at r2 (raw file):

Previously, jeremie-stratumn (Jeremie Dayan) wrote…

I had all sorts of problems on travis and semaphore CI:
https://semaphoreci.com/stratumn/go-indigocore/branches/jd-elasticsearchstore/builds/1
https://travis-ci.org/stratumn/go-indigocore/builds/349873828?utm_source=github_status&utm_medium=notification

the travis error means that it takes too long to even start elasticsearch container..

:'(

---

Comments from Reviewable

[jrmdayn]

Review status: all files reviewed at latest revision, 8 unresolved discussions, some commit checks failed.

---

elasticsearchstore/api.go, line 265 at r2 (raw file):

Previously, bejito (Mourad BEJI) wrote…

Shouldn't we directly paginate the call to ES ?

I dont know that this is possible...
https://discuss.elastic.co/t/pagination-in-aggregation/79423
elastic/elasticsearch#4915
elastic/elasticsearch#21487

---

Comments from Reviewable

[bejito]

elasticsearchstore/api.go, line 265 at r2 (raw file):

Previously, jeremie-stratumn (Jeremie Dayan) wrote…

I dont know that this is possible...
https://discuss.elastic.co/t/pagination-in-aggregation/79423
elastic/elasticsearch#4915
elastic/elasticsearch#21487

indeed...

---

Comments from Reviewable

[alexppxela]

Great!

---

Reviewed 7 of 8 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 11 unresolved discussions, some commit checks failed.

---

elasticsearchstore/api.go, line 291 at r2 (raw file):

	// prevLinkHash filter
	if filter.PrevLinkHash != nil {
		if len(*filter.PrevLinkHash) > 0 {

nit: it's more usual to test if *filter.PrevLinkHash != "" in go than len(...) > 0

---

elasticsearchstore/api.go, line 302 at r2 (raw file):

	// process filter
	if len(filter.Process) > 0 {

Idem for checking emptiness

---

elasticsearchstore/elasticsearchstore.go, line 141 at r2 (raw file):

// AddStoreEventChannel implements github.com/stratumn/go-indigocore/store.Adapter.AddStoreEventChannel
func (es *ESStore) AddStoreEventChannel(eventChan chan *store.Event) {
	es.eventChans = append(es.eventChans, eventChan)

eventChans is not concurrent protected. It is the case for all stores...
AddStoreEventChannel is only called at storehttp startup, there is no race condition but we have to keep it in mind for later (check trace usage)

---

Comments from Reviewable

[jrmdayn]

Review status: 4 of 8 files reviewed at latest revision, 10 unresolved discussions, some commit checks failed.

---

elasticsearchstore/api.go, line 291 at r2 (raw file):

Previously, alexppxela (Alexandre Thibault) wrote…

nit: it's more usual to test if *filter.PrevLinkHash != "" in go than len(...) > 0

done

---

elasticsearchstore/api.go, line 302 at r2 (raw file):

Previously, alexppxela (Alexandre Thibault) wrote…

Idem for checking emptiness

done

---

elasticsearchstore/elasticsearchstore.go, line 68 at r1 (raw file):

Previously, jeremie-stratumn (Jeremie Dayan) wrote…

I see, I'll change that

done

---

elasticsearchstore/elasticsearchstore.go, line 205 at r1 (raw file):

Previously, such (Adrien Montfort) wrote…

No you're right. Right now I don't think there's a way to test only the LinkStore. But the KVStore is only used in tmpop and we have a generic implementation.

ok

---

elasticsearchstore/elasticsearchstore.go, line 141 at r2 (raw file):

Previously, alexppxela (Alexandre Thibault) wrote…

eventChans is not concurrent protected. It is the case for all stores...
AddStoreEventChannel is only called at storehttp startup, there is no race condition but we have to keep it in mind for later (check trace usage)

ok!

---

Comments from Reviewable

[alexppxela]

---

Reviewed 4 of 4 files at r3.
Review status: all files reviewed at latest revision, 8 unresolved discussions.

---

Comments from Reviewable

[alexppxela]

👍

[such]

---

Reviewed 4 of 4 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved, all commit checks successful.

---

Comments from Reviewable