feat(kuma-cp) dp sync tracker runs only one watchdog #731
Conversation
jakubdyszkiewicz
force-pushed
the
builtin-expiration
branch
from
d9193a6
to
5047bf5
Compare
jakubdyszkiewicz
force-pushed
the
dp-sync-tracker-fix
branch
from
5ba6394
to
cfb66e8
Compare
| Expect(err).ToNot(HaveOccurred()) | ||
|
|
||
| // then only one watchdog is active | ||
| time.Sleep(50 * time.Millisecond) // wait for goroutines |
There was a problem hiding this comment.
Is there a better way to syncronize here? Can this create flaky tests when running on slow instances in the CI?
There was a problem hiding this comment.
I tried to do it with channels, but the // then watchdog is still active because other stream is opened is hard to do it.
I don't think that it would cause problems, but I changed the simple wait to gomega Eventually() so we will be checking conditions for 5s every 10ms so even slower machines can check it.
jakubdyszkiewicz
force-pushed
the
builtin-expiration
branch
from
5047bf5
to
1b98015
Compare
jakubdyszkiewicz
force-pushed
the
dp-sync-tracker-fix
branch
from
36c437c
to
efd2ba5
Compare
| } | ||
| } | ||
|
|
||
| var _ envoy_xds.Callbacks = &dataplaneSyncTracker{} | ||
|
|
||
| type streams struct { | ||
| watchdogCancel context.CancelFunc | ||
| activeStreams map[int64]bool |
There was a problem hiding this comment.
I thought we already have type alias for int64. Could it be used here as well?
There was a problem hiding this comment.
I moved the sds streamID to core_xds and reused here
| }) | ||
| go t.newDataplaneWatchdog(dataplaneKey, streamID).Start(stopCh) | ||
| dataplaneSyncTrackerLog.V(1).Info("started Watchdog for a Dataplane", "streamid", streamID, "proxyId", id, "dataplaneKey", dataplaneKey) | ||
| t.Lock() |
There was a problem hiding this comment.
Is it okay that checking of existence in line 81 is not atomic with the actual updating of streams? So after the second lock, some associated resource might exist
There was a problem hiding this comment.
OnStreamRequest for given streamID is synchronous so if I understand your question it should be fine
Summary
With ADS the relation was 1 DP = 1 gRPC stream. With SDS that uses Dataplane Sync Tracker the relation is that 1 DP can open many streams. We want to start only one watchdog for DP so we need to which DP the stream belongs. When closing the stream we need to cancel watchdog only if DP has no streams left.