Add Elasticsearch v7 support#1164
Conversation
| } | ||
|
|
||
| CLIClient interface { | ||
| CatIndices(ctx context.Context) (elastic.CatIndicesResponse, error) |
There was a problem hiding this comment.
Indexes and indices are both accepted and widely used plurals of the noun index. Both appear throughout the English-speaking world, but indices prevails in varieties of English from outside North America, while indexes is more common in American and Canadian English. Meanwhile, indices is generally preferred in mathematical, financial, and technical contexts, while indexes is relatively common in general usage.
I just mock whatever ES has.
|
|
||
| type ( | ||
| BulkProcessor interface { | ||
| Stop() error |
There was a problem hiding this comment.
| Stop() error | |
| common.Daemon |
There was a problem hiding this comment.
There is no Start() here. And error is logged. I prefer to leave it.
| if esBulkProcessor == nil { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
when will esBulkProcessor == nil?
There was a problem hiding this comment.
Whenever c.esClient.BulkProcessor().Do(ctx) returns it (I believe in case of error only). Essentially it is a wrapper and I want wrapper to be nil every time when underlying object is nil.
There was a problem hiding this comment.
Whenever
c.esClient.BulkProcessor().Do(ctx)returns it (I believe in case of error only). Essentially it is a wrapper and I want wrapper to benilevery time when underlying object isnil.
is that weird? wrapper over nil?
There was a problem hiding this comment.
It just simplify caller code w/o checking for nil and err there. I think it is ok.
| if esBulkProcessor == nil { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
when will esBulkProcessor == nil?
2 participants
What changed?
Support of ES v7 was added while v6 is still supported. Implementation is v7 performance oriented. All internal APIs use v7 types which are converted to v6 (performance penalty) in case if ES v6 is used.
All development/test artifacts are switched to v7. Although (for backward compatibility) docker image still uses v6 by default. Set
ES_VERSIONenvironment variable tov7or add corresponding config value:... es-visibility: elasticsearch: version: "v7" url: scheme: "http" ...to switch to v7.
Buildkite also runs integration tests with both v6 and v7 until v6 support is completely removed.
Why?
ES6 EOL 2020-11-20 has passed by, and ES7 has number of breaking changes. Most noticeable one is removal of schema type. ES client library has separate branch for v7 which means that v6 and v7 types are incompatible. Most complexity of this PR is about v6 to v7 types conversion.
How did you test it?
Run existing tests and manual run of canary against both (v6 and v7) versions.
Potential risks
Default build of docker image still uses v6.