tests/robustness: Validate all etcd watches opened to etcd #15893
Conversation
serathius
force-pushed
the
robustness-validate-client-watch
branch
3 times, most recently
from
0760345
to
2ef3492
Compare
serathius
force-pushed
the
robustness-validate-client-watch
branch
from
2ef3492
to
f27fa70
Compare
|
|
||
| func (h History) MaxRevision() int64 { | ||
| var maxRevision int64 | ||
| for _, op := range h.successful { |
There was a problem hiding this comment.
Do we need to handle a situation where len(h.successful) == 0?
There was a problem hiding this comment.
We can maintain special meaning of revision=0 as it's in etcd. Having 0 is still correct as not all clients make KV operations.
| logger.go:130: 2023-05-15T17:42:37.792+0200 INFO Saving operation history {"path": "/tmp/TestRobustness_ClusterOfSize3_Kubernetes/client-1/operations.json"} | ||
| logger.go:130: 2023-05-15T17:42:37.793+0200 INFO Saving watch responses {"path": "/tmp/TestRobustness_ClusterOfSize3_Kubernetes/client-2/watch.json"} |
There was a problem hiding this comment.
Is the change accidentally updated? :/
There was a problem hiding this comment.
No, it's intentional as file paths change.
| } | ||
|
|
||
| func validateGotAtLeastOneProgressNotify(t *testing.T, memberId string, responses []traffic.WatchResponse, expectProgressNotify bool) { | ||
| func validateGotAtLeastOneProgressNotify(t *testing.T, reports []traffic.ClientReport, expectProgressNotify bool) { |
There was a problem hiding this comment.
Is it expected only one out of x watch clients get one progress notify.
I think all x watch clients should get one, respectively because otherwise, that does not sound right from k8s prospective.
There was a problem hiding this comment.
That's not right, manual progress notify is sent back to client requesting it.
There was a problem hiding this comment.
etcd/server/etcdserver/api/v3rpc/watch.go
Line 322 in efcbd95
There was a problem hiding this comment.
I see. expectProgressNotify true means either manual requested or periodic.
Is my statement true for the periodic one?
There was a problem hiding this comment.
Only if all watchers watch a event stream that didn't have any event for required amount of time.
Signed-off-by: Marek Siarkowicz <siarkowicz@google.com>
serathius
force-pushed
the
robustness-validate-client-watch
branch
from
f27fa70
to
6429f47
Compare
cc @ahrtr @ptabor @jmhbnz @chaochn47