Skip to content

Make sure when --es.num-replicas=0, ES indices have number_of_replica=0 not 1 #371

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 165 commits into from
Closed

Make sure when --es.num-replicas=0, ES indices have number_of_replica=0 not 1 #371

wants to merge 165 commits into from

Conversation

bharat-p
Copy link
Member

@bharat-p bharat-p commented Sep 2, 2017 •
edited by yurishkuro
Loading

Resolves #369

ES number of replicas was getting set to 1 even when specifying --es.num-replicas=0 when starting collector and query.

Before fix:

output of localhost:9200/_cluster/health?pretty

{
  "cluster_name" : "jaeger-cluster",
  "status" : "yellow",
  "timed_out" : false,
  "number_of_nodes" : 1,
  "number_of_data_nodes" : 1,
  "active_primary_shards" : 4,
  "active_shards" : 4,
  "relocating_shards" : 0,
  "initializing_shards" : 0,
  "unassigned_shards" : 4,
  "delayed_unassigned_shards" : 0,
  "number_of_pending_tasks" : 0,
  "number_of_in_flight_fetch" : 0,
  "task_max_waiting_in_queue_millis" : 0,
  "active_shards_percent_as_number" : 50.0
}

ES cluster health check shows status 'yellow' and you will notice active_primary_shards=4 and unassigned_shards=4, which means some indexes are configured with value replicas=1, this can be confirmed by checking output of :

http://localhost:9200/jaeger-span-2017-09-02/_settings?pretty

{
  "jaeger-span-2017-09-02" : {
    "settings" : {
      "index" : {
        "mapping" : {
          "nested_fields" : {
            "limit" : "50"
          }
        },
        "number_of_shards" : "1",
        "provided_name" : "jaeger-span-2017-09-02",
        "mapper" : {
          "dynamic" : "false"
        },
        "creation_date" : "1504368267533",
        "requests" : {
          "cache" : {
            "enable" : "true"
          }
        },
        "number_of_replicas" : "1",
        "uuid" : "AkWNKfQSRlCa9pIoxilj5g",
        "version" : {
          "created" : "5050299"
        }
      }
    }
  }
}

Verified after fix:
output of http://localhost:9200/_cluster/health?pretty

{
  "cluster_name" : "jaeger-cluster",
  "status" : "green",
  "timed_out" : false,
  "number_of_nodes" : 1,
  "number_of_data_nodes" : 1,
  "active_primary_shards" : 10,
  "active_shards" : 10,
  "relocating_shards" : 0,
  "initializing_shards" : 0,
  "unassigned_shards" : 0,
  "delayed_unassigned_shards" : 0,
  "number_of_pending_tasks" : 0,
  "number_of_in_flight_fetch" : 0,
  "task_max_waiting_in_queue_millis" : 0,
  "active_shards_percent_as_number" : 100.0
}

After fix output of http://localhost:9200/jaeger-span-2017-09-02/_settings?pretty

{
  "jaeger-span-2017-09-02" : {
    "settings" : {
      "index" : {
        "mapping" : {
          "nested_fields" : {
            "limit" : "50"
          }
        },
        "number_of_shards" : "5",
        "provided_name" : "jaeger-span-2017-09-02",
        "mapper" : {
          "dynamic" : "false"
        },
        "creation_date" : "1504369115915",
        "requests" : {
          "cache" : {
            "enable" : "true"
          }
        },
        "number_of_replicas" : "0",
        "uuid" : "Ou2TIOavSCyIDDqlj_4Gxg",
        "version" : {
          "created" : "5050299"
        }
      }
    }
  }
}

@yurishkuro
Copy link
Member

Personally I would rather have a parameter called "replication factor" that defines the total number of copies of the data, similar to Cassandra. Then the setting es.numReplicas = replicationFactor-1 could be done internally, which would allow RF=0 to be treated as empty value and overwritten by a default, since RF=0 is never valid (while NumReplicas=0 is valid).

I know that this would go against the nomenclature of ES configuration, thus a bit controversial.

@bharat-p
Copy link
Member Author

bharat-p commented Sep 2, 2017

@yurishkuro that should be ok, but as you already pointed out it goes against ES' nomenclature and might be confusing to ES users, also will cause issue with folks who are already relying on this parameter.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 9984acd on bharat-p:es-replica-fix-#369 into 413942a on uber:master.

@yurishkuro
Copy link
Member

maybe we can keep the command line switch name & meaning the same but default it to something like -1 if not specified, and handle the rest internally (like converting internal option to "rf" instead of "numReplicas"). My main concern with this change is that it breaks the convention in Jaeger codebase (and in Go language in general) that empty value (0) can/will be replaced by a default. After this change the value of 0 starts having a meaning rather than being an empty value.

@bharat-p
Copy link
Member Author

bharat-p commented Sep 4, 2017

Another option is we change default value for es.num-replicas to 0 instead of current 1.
cc @mh-park as he is the original author of ES storage plugin for his opinion.

pavolloffay and others added 19 commits September 5, 2017 14:31
@pavolloffay @vprithvi
Makefile use GNU sed and force thrift to use the same user (jaegertra…
6f5941d 
…cing#377)

* Makefile use  GNU sed and force thrift to use the same user

* sed hack to run -i in BSD and GNU version

* try -i.bak workaround

* Add comment to makefile
@yurishkuro
Add Pavol Loffay to CODEOWNERS (jaegertracing#381)
d94ab57 
Resolves jaegertracing#380
@isaachier @black-adder
Add minor doc fixes. (jaegertracing#383)
5d5a16b
@jpkrohling @yurishkuro
Document how to run the agent via Docker with a remote collector (jae…
7abb600 
…gertracing#388)
@pavolloffay @yurishkuro
Add xdock tests for Zipkin json and thrift (jaegertracing#378)
d60f25c
@pavolloffay @yurishkuro
Remove thrift container when it is done (jaegertracing#382)
d17b468
@yurishkuro @black-adder
Add GrafanaLabs and FarmersEdge (jaegertracing#390)
4fbabcc
@pavolloffay @yurishkuro
Do not tag Dicker images with SHA and travis build number tags (jaege…
f0193c6 
…rtracing#376)

except for crossdock images
@yurishkuro
Update to Apache 2.0 License (jaegertracing#391)
d16de31 
* Update to Apache 2.0 License

Per https://github.com/uber/jaeger/issues/252

* update in actual files
@ejwood79 @yurishkuro
Adding Under Armour to adopters (jaegertracing#393)
e156f19 
We're expanding our production deployment of jaeger at UA!
@jnewmano @yurishkuro
Add Weave to list of adopters (jaegertracing#396)
b8a4fe0
@pavolloffay @black-adder
Update docs: quickstart enable zipkin (jaegertracing#398)
651c8ce
Add DCO and updade CONTRIBUTING.md accordingly
09fe054
@yurishkuro
Fix crossdock test-driver upload to Hub (jaegertracing#400)
939dc91
@caniszczyk @yurishkuro
Mention CNCF in README (jaegertracing#402)
f259ee2 
This is required of CNCF projects.
@yurishkuro
Undo image path changes that broke the docs
721241c
@yurishkuro
Define import path in preparation of repository move to jaegertracing (…
489d7ed 
…jaegertracing#408)
@NeoCN @yurishkuro
Fix log field name for health check status (jaegertracing#413)
bb5f150
@yurishkuro
Add CNCF logo
0e3c4cb
yurishkuro and others added 25 commits February 7, 2018 15:53
@yurishkuro
Preparing release 1.2.0 (jaegertracing#689)
dc6c4a7 
Signed-off-by: Yuri Shkuro <ys@uber.com>
@pavolloffay
Collect metrics from ES bulk service (jaegertracing#688)
ef7a367 
* Collect ES bulk metrics

Signed-off-by: Pavol Loffay <ploffay@redhat.com>

* Fix tests

Signed-off-by: Pavol Loffay <ploffay@redhat.com>
@oiooj @black-adder
Export model and protocol type (jaegertracing#691)
9fe8cc8 
The `ProcessorConfiguration` can be exported, but the custom type
`model` and `protocol` can not, so it's hard to new a
`ProcessorConfiguration` outside the pacaage.

Signed-off-by: kun <oiooj@qq.com>
@pavolloffay
Add Dockerfile for hotrod example app (jaegertracing#694)
a631623 
* Binding all interfaces on hotrod

Signed-off-by: Guilherme Baufaker Rêgo <gbaufake@redhat.com>

* Hotrod Containerized on Docker

Signed-off-by: Guilherme Baufaker Rêgo <gbaufake@redhat.com>

* Moving HotRod Containerized to Hotrod folder

and some minor adjustments on Dockerfile

Signed-off-by: Guilherme Baufaker Rêgo <gbaufake@redhat.com>

* Copy only Hotrod binary to Dockerfile

- exposing more ports on Dockerfile
and copy only binary to Dockerfile

Signed-off-by: Guilherme Baufaker Rêgo <gbaufake@redhat.com>
@zkanda @yurishkuro
Rename MultiplexWriter to CompositeWriter (jaegertracing#701)
2ad1e59 
Signed-off-by: zkanda <hi@zkanda.io>
@yurishkuro
Add a flag to control Cassandra consistency level (jaegertracing#700)
2557fa1 
Signed-off-by: Yuri Shkuro <ys@uber.com>
@pavolloffay @yurishkuro
Pin testify version to ^1.2.1 (jaegertracing#710)
b1d39bb 
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
@oiooj @black-adder
Add to WaitGroup before the goroutine creation (jaegertracing#711)
e2f1150 
The calls to Add should execute before the statement creating the
goroutine or other event to be waited for.

Signed-off-by: kun <oiooj@qq.com>
@vprithvi @yurishkuro
When converting microseconds to time.Time ensure UTC timezone (jaeger…
d59f917 
…tracing#712)


Signed-off-by: Prithvi Raj <p.r@uber.com>
@yurishkuro
Test that processes with identical tags are deduped (jaegertracing#708)
b61b7ec 
Signed-off-by: Yuri Shkuro <ys@uber.com>
@black-adder
Add sampling handler and sampling strategy store (jaegertracing#674)
d5ad797
@yurishkuro
Make sure different tracers in HotROD process use different random ge…
25a1c56 
…nerator seeds (jaegertracing#718)

Signed-off-by: Yuri Shkuro <ys@uber.com>
@eundoosong @pavolloffay
fix typo in healthcheck (jaegertracing#721)
62b3b9c 
Signed-off-by: Eundoo Song <eundoo.song@gmail.com>
@black-adder
Add sampling handler to main (jaegertracing#720)
ba7d67b
@yurishkuro
Move static handler registration into a shared function (jaegertracin…
2eeffec 
…g#725)

Signed-off-by: Yuri Shkuro <ys@uber.com>
@yurishkuro
Avoid duplicate expvar metrics - fixes jaegertracing#716 (jaegertraci…
cce04e7 
…ng#726)

* Use different namespaces for Cassandra primary and archive storage
* Fix a bug where archive factory wasn't used in query/main
* Use new expvar metrics module from jaeger-lib 1.4.0

Signed-off-by: Yuri Shkuro <ys@uber.com>
@jpkrohling @yurishkuro
Disable double compression on Prom Handler - fixes jaegertracing#697 (j…
5d4fed6 
…aegertracing#735)

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
@yurishkuro
Apply namespace when creating Prometheus metrics factory (fix for jae…
6fce4f9 
…gertracing#732) (jaegertracing#733)

Signed-off-by: Yuri Shkuro <ys@uber.com>
@ledor473 @yurishkuro
Use the default metricsFactory if not provided (jaegertracing#739)
ff666cd 
Signed-off-by: Louis-Etienne Dorval <louis-etienne.dorval@ticketmaster.com>
@pavolloffay
Publish hotrod image to docker hub (jaegertracing#702)
30d3e18 
* Publish hotrod to dockerub

Signed-off-by: Pavol Loffay <ploffay@redhat.com>

* Fix review comments

Signed-off-by: Pavol Loffay <ploffay@redhat.com>

* Add env to dockerfile to handle collector hostport

Signed-off-by: Pavol Loffay <ploffay@redhat.com>

* Use alpine

Signed-off-by: Pavol Loffay <ploffay@redhat.com>

* Use scratch and entrypoint hotrod all

Signed-off-by: Pavol Loffay <ploffay@redhat.com>

* Remove cd -

Signed-off-by: Pavol Loffay <ploffay@redhat.com>

* Fix cd

Signed-off-by: Pavol Loffay <ploffay@redhat.com>

* Add ls

Signed-off-by: Pavol Loffay <ploffay@redhat.com>

* Remove bindata.go

Signed-off-by: Pavol Loffay <ploffay@redhat.com>

* Add wait until 200

Signed-off-by: Pavol Loffay <ploffay@redhat.com>
@pavolloffay
Dockerize es-indices-clean script (jaegertracing#741)
5c09726 
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
@pavolloffay @yurishkuro
Publis hotrod only when TRAVIS_SECURE_ENV_VARS is true (jaegertracing…
37e6430 
…#753)

Signed-off-by: Pavol Loffay <ploffay@redhat.com>
@bharat-p
Make sure when --es.num-replicas=0, ES indices have number_of_replica…
cff2da2 
…s=0 not 1 (jaegertracing#369)
@bharat-p
Fix ES spanstore writer test. (jaegertracing#369)
bd8fb9b
@bharat-p
Merge latest upstream
2d00f04
@bharat-p bharat-p requested a review from pavolloffay as a code owner March 22, 2018 23:45
@bharat-p bharat-p closed this Mar 22, 2018
@bharat-p bharat-p deleted the es-replica-fix-#369 branch March 22, 2018 23:46
@bharat-p bharat-p mentioned this pull request Mar 23, 2018
Merged
@yurishkuro
Copy link
Member

There's a new PR #754, but the discussion above was left unresolved.

@pavolloffay Since you were doing work with ES and its configuration, do you have an opinion?

cc @manannayak

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@black-adder black-adder Awaiting requested review from black-adder

@oibe oibe Awaiting requested review from oibe

@vprithvi vprithvi Awaiting requested review from vprithvi

@yurishkuro yurishkuro Awaiting requested review from yurishkuro

@pavolloffay pavolloffay Awaiting requested review from pavolloffay

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ES replicas defaults to 1 even when specifying --es.num-replicas=0 when starting collector and query