before:
go test -v -race -count 1 -run ^TestWatchNotHangingOnStartupFailure$
ok k8s.io/apiserver/pkg/storage/cacher 6.775s
after:
go test -v -race -count 1 -run ^TestWatchNotHangingOnStartupFailure$
ok k8s.io/apiserver/pkg/storage/cacher 2.781s
Kubernetes-commit: f5d945eb43c7bf8036a4bad8c22448e1146a7498
The individual cases can be safely run in parallel.
Before
go test -race -run TestWaitUntilWatchCacheFreshAndForceAllEvents
ok k8s.io/apiserver/pkg/storage/cacher 10.787s
After:
go test -race -run TestWaitUntilWatchCacheFreshAndForceAllEvents
ok k8s.io/apiserver/pkg/storage/cacher 4.857s
Kubernetes-commit: 3ecbb4dee00a5dd1e43e24a5952c2a90ef507ef1
updates the test to wait 300 ms instead of 3s
the watch was established otherwise
we would be blocking on a call to cache.Watch(...)
in addition to that, the tests are serial in nature,
meaning that there is no other actor
that could add items to the database,
which could result in receiving new items.
Before:
go test -race -run TestEmptyWatchEventCache
ok k8s.io/apiserver/pkg/storage/cacher 8.450s
After:
go test -race -run TestEmptyWatchEventCache
ok k8s.io/apiserver/pkg/storage/cacher 2.635s
Kubernetes-commit: 926122c035a4f47a880db24d1a0be7ec129dd44d
It's possible that the watcher is already not in the structure (e.g. in case of
simultaneous Stop() and terminateAllWatchers(), but it is safe to call stopLocked()
on a watcher multiple times.
Kubernetes-commit: 7e35823690df01bd019a88d3346bd3ac820afaca
It's possible that the watcher is already not in the structure (e.g. in case of
simultaneous Stop() and terminateAllWatchers(), but it is safe to call stopLocked()
on a watcher multiple times.
Kubernetes-commit: bbca4a4b9add0f6c58e132500fd89dd39ee077f4
Request bookmark every 100ms when there is at least one request blocked on revision not present in watch cache.
Kubernetes-commit: 39bb8f4bb1d013937aceac6c387563ffe13545c5
This commit prepares for when cacher tests are moved here
from the `tests` package. Tests in that package redeclare
some of the testing utils that exist here, so dedup-ing them.
This commit also adapts to any changes in test util signatures.
There are still some utils that can be reused but currently are
highly specific to some tests. (ex: watch_cache_test.go)
Signed-off-by: Madhav Jivrajani <madhav.jiv@gmail.com>
Kubernetes-commit: 70978e4af619819787a4eb544ffd732aa7954d76
If the cacher hasn't seen any event (when lastProcessedResourceVersion is zero) and
the bookmarkTimer has ticked then we shouldn't popExpiredWatchers. This is
because the watchers wont' be re-added and will miss future bookmark events when
the cacher finally receives an event via the c.incoming chan.
Kubernetes-commit: 6db4cbfde7babfb34f5cd1059c769ec2d870f12a
* cacher: remove locking from watcherBookmarkTimeBuckets
it turns out that the watcherBookmarkTimeBuckets
is called from only three places/methods: startDispatching, finishDispatching and Watch.
All these methods acquire c.Lock() before touching watcherBookmarkTimeBuckets.
Thus we could remove explicit locking in
watcherBookmarkTimeBuckets since the access is already synced.
* cacher: rename watcherBookmarkTimeBuckets methods to indicate that proper synchronisation must be used
Kubernetes-commit: eab66a687b282266f0520b79166f7f55828ffd28
waitUntilWatchCacheFreshAndForceAllEvents must be called without
a read lock held otherwise the watchcache won't be able to make
progress (i.e. the watchCache.processEvent method that requries acquiring an exclusive lock)
the deadlock can happen only when the alpha watchlist feature flag is on
and the client specifically requests streaming.
Kubernetes-commit: 476e407ffd2ab393840d3f7a9fd01b71698738a3
* ftr(watch-cache): add benchmarks
* ftr(kube-apiserver): faster watch-cache getlist
* refine: testcase name
* - refine var name make it easier to convey meaning
- add comment to explain why we need to apply for a slice of runtime.Object instead of making a slice of ListObject.Items directly.
Kubernetes-commit: 75f17eb38fc8bbcb360d43dffce6e27a7159d43f
this method waits until cache is at least
as fresh as given requestedWatchRV if sendInitialEvents was requested.
Additionally, it instructs the caller whether it should ask for
all events from the cache (full state) or not.
Kubernetes-commit: 21fb98105043d1a15ef48089ef231931851d2d15
* cacher allow context cancellation if not ready
Replace the sync.Cond variable with a channel so we can use the
context cancellation signal.
Co-authored-by: Wojciech Tyczy<C5><84>ski <wojtekt@google.com>
Change-Id: I2f75313a6337feee440ece4c1e873c32a12560dd
* wait again on pending state
Change-Id: I1ad79253a5a5d56a4d9611125825b1f7ad552be8
---------
Co-authored-by: Wojciech Tyczy<C5><84>ski <wojtekt@google.com>
Kubernetes-commit: 3b17aece1fa492e98aa82b948597b3641961195f
The original design was to honour strong consistency
semantics for when the RV is unset, i.e. serve the
watch by doing a quorum read.
However, the implementation did not match the intent,
in that, the Cacher did not distinguish between set
and unset RV. This commit rectifies that behaviour by
serving the watch from the underlying storage if the
RV is unset.
This commit subsequently also adds a test for the same.
Signed-off-by: Madhav Jivrajani <madhav.jiv@gmail.com>
Kubernetes-commit: 610b67031c79c6c38964631d27dd59df357c6d2e
This commit allows injecting errors for the
Watch() method of the dummy storage impl.
As a consequence of this, a race is introduced
between when the injected error is written and
read whenever a Watch() is invoked using the
dummy storage. This commit adds locking in order
to mitigate this.
Signed-off-by: Madhav Jivrajani <madhav.jiv@gmail.com>
Kubernetes-commit: 2593671337ad449f51b9dcc0b63aa190dd07ab68
Use the group resource instead of objectType in watch cache metrics,
because all CustomResources are grouped together as
*unstructured.Unstructured, instead of 1 entry per type.
Signed-off-by: Andy Goldstein <andy.goldstein@redhat.com>
Kubernetes-commit: d08b69e8d35a5aa73a178c508f9b0e1ad74b882d
All CustomResources are treated as *unstructured.Unstructured, leading
the watch cache to log anything related to CRs as Unstructured. This
change uses the schema.GroupResource instead of object type for all type
related log messages in the watch cache, resulting in distinct output
for each CR type.
Signed-off-by: Andy Goldstein <andy.goldstein@redhat.com>
Kubernetes-commit: 397533a4c2df9639ff4422c907d06fae195a1835
Using a Pod type in a GetList() call in a test
can panic at worst and error out at best. Here,
neither happened because the error condition
being tested for (cacher being stopped or not)
gets returned before the list pointer can be
enforced.
This commit changes the above to use PodList.
Signed-off-by: Madhav Jivrajani <madhav.jiv@gmail.com>
Kubernetes-commit: 487761f4e2543114db158f0d59e598dedc481882
The cacher blocks requests until it is ready, however, the
ready variable doesn't differentiate if the cacher was stopped.
The cacher is using a condition variable based on sync.Cond to
handle the readiness, however, this was not taking into account
if it was not ready because it was waiting to be ready or it was
stopped.
Add a new condition to the condition variable to handle the
stop condition, and returning an error to signal the goroutines
that they should stop waiting and bail out.
Kubernetes-commit: 2cb3a56e83ae33464edb174b1b6373ba50600759
* Remove linter warnings.
* Cancel contexts to avoid leaks.
* Rename a few XXXThreadUnsafe to XXXLocked to
maintain consistency.
* A few are still called XXXThreadUnsafe mainly
because those are safe to be called from the
perspective that only one gorotuine will access
them - not really called under a lock.
Signed-off-by: Madhav Jivrajani <madhav.jiv@gmail.com>
Kubernetes-commit: c3081b48759db1f05a446f2acca7e05c4511ce2e
- Modify GetAllEventsSinceThreadUnsafe to return a watchCacheInterval
- Modify Watch() to compute a watchCacheInterval rather than a slice
of all "initEvents" and pass this interval to process()
- Use interval::Next() to obtain events to process rather than obtain
them all at once
- Modify tests accordingly to use interval
- On invalidation, stop processing and stop the watch.
- Make indexValidator injectable for testing
- Add unit test for verifying the behaviour of stopping the watch.
Signed-off-by: Madhav Jivrajani <madhav.jiv@gmail.com>
Kubernetes-commit: 7f2aa7ad3a61a52d0a780f904b291d063399c28a
Split process() function into processEvents() and process().
This is done in anticipation of GetAllEventsSinceThreadUnsafe()
returning an entity using which events can be constructed and
not the events itself.
Subsequently, this commit also moves updating resource version
for initEvents from Watch() to the processEvents() func.
Signed-off-by: Madhav Jivrajani <madhav.jiv@gmail.com>
Kubernetes-commit: aab7cd3d8a66f425022ca5b2a2bd0d3019efe526