stellar / helm-charts

Helm charts for deploying SDF maintained software
3 stars 12 forks source link

Changed rpc statefulset to deployment #81

Closed sreuland closed 3 months ago

sreuland commented 3 months ago

To reduce the potential for concurrent access of old/new pods during upgrade strategies of statefulset, we are converting to a deployment for more deterministic Recreate strategy to serialize the old/new pod lifecycle during upgrade.

Closes #79

tested the chart with template:

cat << EOF | helm template soroban-rpc-futurenet-prd ./charts/soroban-rpc --values https://raw.githubusercontent.com/stellar/helm-charts/main/charts/soroban-rpc/values.yaml --values=- >soroban-rpc-futurenet-prd.yml
sorobanRpc:
  ingress:
    ingressClassName: public
    host: soroban-futurenet.stellar.org
  annotations:
    prometheus.io/port: "6061"
    prometheus.io/scrape: "true"
  labels:
    rpc_network: "futurenet"
  resources:
    limits:
      cpu: 1         # Bumped from 250m
      memory: 2560Mi # Bumped from 512Mi
    requests:
      cpu: 50m      # Default
      memory: 384Mi # Bumped from 128Mi
  persistence:
    enabled: true
    storageClass: default
    size: 200G
EOF
sreuland commented 3 months ago

It would be good to avoid Deployment because it requires manual PVC and this prevents horizontal scaling. We'd need to convert back to StatefulSet anyway when soroban-rpc supports HA and this would be a disruptive change. Should we run some tests in the SDF k8s cluster to check if it works?

yes, we discussed this more in platform team meeting also, and have identified a potential 'magic bullet' to avoid downtime during a statefulset upgrade by just increasing the replicas of the sts to 2, as that allows for one ordinal pod to always be healthy and routable to the service. And sts rolling upgrade won't require ReadWriteOncePod on the pvc access, because sts controller always terminates an ordinal pod first before it starts a new pod for same ordinal, and pvc usage is per-ordinal.

So, we will try replicas=2 out in dev and if it suffices, can proceed with that or possibly explore a blue/green approach with statefulset rollingupdate partitioning

in either case, will close this pr as it's not needed, thanks.

cc: @aditya1702 @mollykarcher @2opremio

sreuland commented 3 months ago

closing this as obsolete now, in favor of better ideas based on statefulset replicas or partitioning