manticoresoftware / manticoresearch-helm

Helm chart for Manticore Search
Apache License 2.0
34 stars 10 forks source link

Why is balancer is fixed with only one replica? #61

Closed elmariofredo closed 1 year ago

elmariofredo commented 1 year ago

See helm template https://github.com/manticoresoftware/manticoresearch-helm/blob/master/charts/manticoresearch/templates/manticore-balancer.yaml#L10 there is fixed number of replicas to '1'. This doesn't seem to be right as in case of node fail it will disrupt all manticore read operations from our app and therefore cause application outage. If there is no reason to have this fixed to only one instance I can prepare PR to add it as values.yaml variable.

djklim87 commented 1 year ago

Hello. Yes, undoubtedly you are right. In the case of HA, it's bad to have only one balancer's replica. But it's not enough only change count of replicas in configs.

There is an observer system between workers and balancers. After each worker starts it pings the balancer. Balancer receives that call and updates his config. Thus our code expects that the balancer is only one.

I am not sure that this observation will work as we expect within only replicas changes but without code fixes

barryhunter commented 1 year ago

Because of this, I found it best to just use the workers directly and not bother with the balancer for running queries (although its still running as Command and Control)

Use the kubernetes 'service' to load balance between the worker nodes $host = "manticorert-worker-svc.$namespace.svc.cluster.local";

The service automatically adds/removes pods during disruptions (planned or otherwise)

Just define a serviceAccount for the helm chart values: fullNameOverride: manticorert

serviceAccount:
  name: manticorert
elmariofredo commented 1 year ago

Problem is that during adding new worker, before it is fully synced, requests are already passed to this new worker. This is ending with error of missing index.

I have two questions:

  1. Is there any plan to resolve single point of failure with balancer resolved?
  2. Can we scale down balancer to zero instance, to prevent wasting resources?
djklim87 commented 1 year ago

Sorry for the late answer. Yes, we planned to fix balancer replicas. And yes you can scale balancer but it gives you some traffic balancing. If you are sure that you don't need it, you can switch it off

barryhunter commented 1 year ago

before it is fully synced, requests are already passed to this new worker.

Oh, that is a good point! Not an issue I've been worried about, adding new indexes is rare, so unlikely to be during a 'disruption' (planed or otherwise) and if a few very new records are not yet available not too worried. But it could certainly be an issue in some applications.

Suppose ideally readinessProbe could perhaps detect if replication is 'in sync' and if not allow the pod into the service if not. But is then back to duplicating what balancer is doing (supposedly only serving fully replicated indexes)

elmariofredo commented 1 year ago

Suppose ideally readinessProbe could perhaps detect if replication is 'in sync' and if not allow the pod into the service if not. But is then back to duplicating what balancer is doing (supposedly only serving fully replicated indexes)

I wonder what is the benefit of using custom balancer, when in K8s we have all necessary tools to loadbalance traffic?

sanikolaev commented 1 year ago

what is the benefit of using custom balancer, when in K8s we have all necessary tools to loadbalance traffic

The k8s lb doesn't care what it's balancing and can't be as efficient as Manticore's lb. It can't do either of this https://manual.manticoresearch.com/Creating_a_cluster/Remote_nodes/Load_balancing#Load-balancing

barryhunter commented 1 year ago

But the balancer is hardcoded to use plain 'roundrobin' distribution anyway.

I suppose that is slightly better than the effectively 'random' distribution achieved by DNS shuffling in the Service.

To really 'benefit' from manticore based distribution, think would need sharding. Not sure if replication garentees the shard distribution is same on all workers (so that if ballancer knew the shards, it could confidently distribute to workers without missing any documents)

... and/or a way to have the balancer create a combined distributed index. (eg explicitly create 3 separate tables, and and the balancer creates a distributed index for all three, so can be searched in parallel over separate agents. Cant do it manually with balancer.config.content as dont know the node-ips.

sanikolaev commented 1 year ago

What you say, Barry, sounds like the auto-sharding task which we already have specified out (for Manticore Search, not the helm chart), but which is not done yet. Unfortunately it requires changes in the search daemon at least to distribute writes, so it can't be done exclusively in the helm chart or via Buddy.

BTW you can configure HA strategy here

barryhunter commented 1 year ago

Ah sorry, yes, see it is now configurable, nice!. I still using 5.0.20 were it was hard coded in observer.php

Yes, auto-sharding, would make balancer truly useful :)

But it could also be 'manual' if there was a way to configure it on balancer, so could define something like on the balancer

sharded-index: index1[index1A,index1B,index1C]

and the balancer would auto expand that out to a distributed index...

Ie the three component indexes are separate RT indexes in their own right (just distributed to all three nodes)

index index1
{
        type = distributed
        ha_strategy = nodeads
        agent = 10.72.34.177:9312:index1A|10.72.35.160:9312:index1A|10.72.40.187:9312:index1A
        agent = 10.72.35.160:9312:index1A|10.72.34.177:9312:index1A|10.72.40.187:9312:index1A
        agent = 10.72.40.187:9312:index1A|10.72.35.160:9312:index1A|10.72.34.177:9312:index1A
}

Which will 'normally' be searched in parallel over all three agent/nodes. This is entirely done in the balancer. Although the application would have to know not to write direct to index1, would have to write direct to the relevant shard.

djklim87 commented 1 year ago

Done in https://github.com/manticoresoftware/manticoresearch-helm/pull/62

sanikolaev commented 1 year ago

The docs have been updated - https://github.com/manticoresoftware/manticoresearch-helm#variables

elmariofredo commented 1 year ago

Thanks a lot for the fix 👍