Skip to content

Drop replicas for kind setup for valkey#267

Open
Nina Polshakova (npolshakova) wants to merge 1 commit into
agent-substrate:mainfrom
npolshakova:fix-valkey-drop-replicas
Open

Drop replicas for kind setup for valkey#267
Nina Polshakova (npolshakova) wants to merge 1 commit into
agent-substrate:mainfrom
npolshakova:fix-valkey-drop-replicas

Conversation

@npolshakova

Copy link
Copy Markdown
Contributor

Fixes #225
(Or at least reduce the cost to spin it up in kind)

It's a good idea to open an issue first for discussion.

  • Tests pass
  • Appropriate changes to documentation are included in the PR

Signed-off-by: npolshakova <nina.polshakova@solo.io>
@npolshakova

Copy link
Copy Markdown
Contributor Author

Benjamin Elder (@BenTheElder) you mentioned this might affect test coverage, so if 3 replicas isn't enough I can maybe just make it configurable in the script?

@BenTheElder

Copy link
Copy Markdown
Collaborator

Yeah -=- I think we were concerned that shards would change behavior (need to be careful not to attempt to query across shards) but probably not replication. We may want to only disable replication in the local testing, and I'm not sure about shard count or more generally what is desirable.

I do want to avoid a lot of skew between local and "real" deployments where possible but I'm somewhat skeptical that we need redundant copies in memory for local dev ..

cc Grant McCloskey (@MushuEE) Julian Gutierrez Oschmann (@juli4n)

Benjamin Elder (BenTheElder) pushed a commit that referenced this pull request Jun 17, 2026
Fixes #225

- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR

Tested local setup:
```
❯ kubectl exec -n ate-system valkey-cluster-0 -- cat /tmp/valkey.conf | grep cluster-announce
cluster-announce-hostname valkey-cluster-0.valkey-cluster-service.ate-system.svc
cluster-announce-tls-port 6379
cluster-announce-bus-port 16379
```

Separate from this, we should decrease the number of clusters, because
right now all of these get created:
```
❯ kubectl exec -n ate-system valkey-cluster-0 -- valkey-cli --tls --cacert /etc/valkey-ca/ca.crt --cert /run/servicedns.podcert.ate.dev/credential-bundle.pem --key /run/servicedns.podcert.ate.dev/credential-bundle.pem cluster nodes
d6ca3f519ddcf774bc8df68f7b7ab6c82cc35066 10.244.0.25:6379@16379,valkey-cluster-2.valkey-cluster-service.ate-system.svc master - 0 1781721223000 3 connected 10923-16383
6f66ad32594c1a5dc550729434996eb129112a1f 10.244.0.27:6379@16379,valkey-cluster-3.valkey-cluster-service.ate-system.svc slave d6ca3f519ddcf774bc8df68f7b7ab6c82cc35066 0 1781721224614 3 connected
c72d9e9bd165b0f800a135fbda1d8184e8111319 10.244.0.28:6379@16379,valkey-cluster-4.valkey-cluster-service.ate-system.svc slave 1c37cdd7fdcc0a261614a6e3936cc0043fe63d35 0 1781721223594 1 connected
54690be32b6647d43e5e1171e6db92ddf9515ad7 10.244.0.20:6379@16379,valkey-cluster-1.valkey-cluster-service.ate-system.svc master - 0 1781721224614 2 connected 5461-10922
6d862697f8af8750a5280409d1eb2bce9f3f8e0a 10.244.0.26:6379@16379,valkey-cluster-5.valkey-cluster-service.ate-system.svc slave 54690be32b6647d43e5e1171e6db92ddf9515ad7 0 1781721223594 2 connected
1c37cdd7fdcc0a261614a6e3936cc0043fe63d35 10.244.0.22:6379@16379,valkey-cluster-0.valkey-cluster-service.ate-system.svc myself,master - 0 0 1 connected 0-5460
```

#267 is a quick fix to
decrease the pain of setting up the cluster + tearing it down. This
would fix the stale IP issue.

Signed-off-by: npolshakova <nina.polshakova@solo.io>
@BenTheElder

Copy link
Copy Markdown
Collaborator

Needs a rebase after the other PR, thanks! Raised in the community meeting today for input.

@MushuEE

Copy link
Copy Markdown
Collaborator

There will still be crosslot concerns without replicas. But is this making 3 replicas with a single shard or 3 shards with no replicas? We are handling cross-shard queries in things like ListActors but those calls have no consistency promises. I think we could allow for "local" vs "real" deployments as the abstraction layers that need scale and "real" testing should enable that.

Can we build an abstraction over the .yaml that allows users to set the replication with a standard default?

@BenTheElder

Copy link
Copy Markdown
Collaborator

Can we build an abstraction over the .yaml that allows users to set the replication with a standard default?

We apply kustomizations to install on kind already, so we can definitely do that. I think we should.

But we should also be careful to make sure it doesn't impact the quality of testing. My rough read is that valkey replication won't impact that much but if we did say, only one cluster shard, that might be too unrealistic.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Valkey statefulset needs to be restarted daily

3 participants