[Garbage Collector] Umbrella: known issues of garbage collector #26120

caesarxuchao · 2016-05-23T22:55:11Z

This is a umbrella issue for Todos and known problems of the garbage collector that we have to solve before graduating GC to beta.

_Functionalities_

[optional]
This race will cause DeleteOptions.OrphanDependents=true not orphan all dependents. We expect this race to be rare. Here are the details: there is no guarantee on the ordering of the events of different resources, it's possible that the GC observes the orphan request (which is an Update event of the owner resource), before observing the creation/update of the dependents. Consequently, these dependents will not be orphaned. [GarbageCollector] monitor the watch latency #30483 adds a monitor the latency of watch, because the latency is small, so this race is rare.
Considered solution:
a. let the GC wait for a short period of time (e.g., 1 min) before carrying out the orphaning procedure, but it won't thoroughly solve the problem, and it will make the deletion slow.
b. let the user supply a resourceVersion, and before carrying out the orphaning procedure, let GC wait until it has observed events of the dependent resource with a larger resourceVersion. Problem is GC is a client, and a client should treat resourceVersion as opaque.
[optional] Need a discovery mechanism to determine what resources GC should manage. For example, GC needs to determine if it should watch for extensions/v1beta1/job or batch/v1/job. (edit: this use case doesn't matter, because the ownerRef will only point to one version of the object, the other version of the object will just be a shadow)
~~[DONE] According to the controllerRef proposal Proposal for ControllerReference #25256, "GarbageCollector will remove ControllerRef from objects that no longer points to existing controllers".~~
~~[DONE] Update at least one controller to use GC. Now replicaset controller and replicationcontroller manager use GC ([GarbageCollector] Let the RC manager set/remove ControllerRef #27600)~~
~~[DONE] [update] we have foreground gc now. Expose the progress of garbage collection. See [RFC][GarbageCollector] expose the progress of garbage collection #29891.~~
[Fixing, see GC: Fix re-adoption race when orphaning dependents. #42938 ] The design doc said before orphaning dependents, GC should wait for the owner's controller to bump up owner's ObservedGeneration, which means the owner controller has acknowledged that it has observed the deletion of the owner and will stop adoption. Otherwise GC's orphaning process will race with owner controller's adoption process, and results in the deletion of the dependents. We hasn't implemented this yet. We expect this race to be rare, because currently only the replicaset and replication controller does the adoption, and it's triggered by updates to RC or the 10-minute resync. We have an e2e test for orphaning 100 pods controlled by a RC, it never hits this race.
[Done for new resources. For old resources 200 is returned for compatiblity] API server should return 202 if a deletion request is not completed synchronously. (API server should return 202 if the operation is asynchronous #33196)
[ tracked in https://github.com/Garbage collector should support non-core APIs #44507] Supporting non-core APIs, either registered via ThirdParty Resource or kube-aggregator.

_Peformance_

[Mechanism is there, need numbers] benchmark the average queuing time (eventQueue and dirtyQueue) ([GarbageCollector] measure latency #28387)
~~2. [Done] Improvement: update the List and Watch to only store the TypeMeta and ObjectMeta ([GarbageCollector] only store typeMeta and objectMeta in the gc store #28480)~~
[Done] [GarbageCollector] add absent owner cache #31167. Caching known deleted UID. GC contacts API server to see if an owner exists when processing its dependents. If the owner doesn't exist according to API server, it won't exist in the future either, because it's impossible for user to predicate UID of a future object and put it in the ownerRef. Such a cache is very useful in the RC-Pods case, because GC will check for the existence of the RC for every pods it created. Such a cache will help API server as well because a GET request with JSON media type is expensive for the API server.
~~4. [RFC] In API server, support resouceVersion as a delete precondition. This will make the Get() in processItem() unnecessary.~~

References:

@lavalamp @gmarek @derekwaynecarr @kubernetes/sig-api-machinery

The text was updated successfully, but these errors were encountered:

hongchaodeng · 2016-05-23T23:20:58Z

GC observes the orphan request (which is an Update event of the owner resource), before observing the creation/update of some dependents

Can you give an example for "Update event of the owner resource", "creation/update of some dependents"?

Where does the event come from? Why aren't they ordered?

caesarxuchao · 2016-05-23T23:30:52Z

@hongchaodeng, Let's use a replication controller and its pods as an example.
In the real time, things happen in this order:

the replication controller R is created;
the replication controller manager creates several pods P1-Pn, which are the dependents of R;
user deletes R with DeleteOptions.OrphanDependents=true and expects P1-Pn are deleted.

But from the GC's perspective, it may observe things in this order:

the replication controller R is created;
user deletes R with DeleteOptions.OrphanDependents=true
the replication controller manager creates several pods P1-Pn, which are the dependents of R;

So the GC doesn't know P1-Pn are dependents of R, so it fails to orphan them.

caesarxuchao · 2016-07-20T22:38:09Z

@deads2k FYI

smarterclayton · 2016-07-21T00:49:18Z

It's reasonable for certain clients to potentially treat resource version as comparable - GC is certainly one of them. We do want to have UID tombstones eventually, but it's certainly a lot of work to get there. What other options do we have?

smarterclayton · 2016-07-21T00:50:32Z

Need a discovery mechanism to determine what resources GC should manage.

As discussed today on sig meeting, we need this for the "migrate client" to touch every object at least once as well.

caesarxuchao · 2016-07-25T23:53:10Z

It's reasonable for certain clients to potentially treat resource version as comparable - GC is certainly one of them. We do want to have UID tombstones eventually, but it's certainly a lot of work to get there. What other options do we have?

@smarterclayton By UID tombstones do you mean etcd3's ability to store versions of deleted objects or constructing our own API?

smarterclayton · 2016-07-26T00:11:26Z

Possibly both.

Automatic merge from submit-queue [GarbageCollector] add absent owner cache  **What this PR does / why we need it**: Reducing the Request sent to the API server by the garbage collector to check if an owner exists. **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes # #26120 **Special notes for your reviewer**: **Release note**:  ```release-note ``` Currently when processing an item in the dirtyQueue, the garbage collector issues GET to check if any of its owners exist. If the owner is a replication controller with 1000 pods, the garbage collector sends a GET for the RC 1000 times. This PR caches the owner's UID if it does not exist according to the API server. This cuts 1/3 of the garbage collection time of the density test in the gce-500 and gce-scale, where the QPS is the bottleneck.

0xmichalis · 2017-03-10T15:10:11Z

The design doc said before orphaning dependents, GC should wait for the owner's controller to bump up owner's ObservedGeneration, which means the owner controller has acknowledged that it has observed the deletion of the owner and will stop adoption.

Is deletion incrementing metadata.Generation?

caesarxuchao · 2017-03-10T18:12:58Z

Is deletion incrementing metadata.Generation?

Yes.

Automatic merge from submit-queue (batch tested with PRs 43106, 43110) Wait for garbagecollector to be synced in test Fix #42952 Without the `cache.WaitForCacheSync` in the test, it's possible for the GC to get a merged event of RC's creation and its update (update to deletionTimestamp != 0), before GC gets the creation event of the pod, so it's possible the GC will handle the foreground deletion of the RC before it adds the Pod to the dependency graph, thus the race. With the `cache.WaitForCacheSync` in the test, because GC runs a single thread to process graph changes, it's guaranteed the Pod will be added to the dependency graph before GC handles the foreground deletion of the RC. Note that this pull fixes the race in the test. The race described in the first point of #26120 still exists.

fejta-bot · 2017-12-31T14:13:35Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

fejta-bot · 2018-01-30T14:21:34Z

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten
/remove-lifecycle stale

fejta-bot · 2018-03-01T15:07:49Z

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

caesarxuchao added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label May 23, 2016

caesarxuchao self-assigned this May 23, 2016

caesarxuchao mentioned this issue May 23, 2016

[GarbageCollector] Add orphaning finalizer logic to GC #25599

Merged

caesarxuchao changed the title ~~Known issues of the garbage collector~~ [Garbage Collector] Unbrella: TODOs of the garbage collector for beta Jul 20, 2016

caesarxuchao changed the title ~~[Garbage Collector] Unbrella: TODOs of the garbage collector for beta~~ [Garbage Collector] Umbrella: TODOs of the garbage collector for beta Jul 20, 2016

caesarxuchao mentioned this issue Jul 29, 2016

[GarbageCollector] Adding a proposal for server-side cascading deletion #23656

Merged

This was referenced Aug 10, 2016

Server-side cascading deletion #19054

Closed

[GarbageCollector] monitor the watch latency #30483

Closed

caesarxuchao changed the title ~~[Garbage Collector] Umbrella: TODOs of the garbage collector for beta~~ [Garbage Collector] Umbrella: known issues of garbage collector Aug 19, 2016

caesarxuchao mentioned this issue Aug 22, 2016

[GarbageCollector] add absent owner cache #31167

Merged

caesarxuchao mentioned this issue Sep 12, 2016

EnableGarbageCollection should be part of generic.RESTOptions #32533

Merged

This was referenced Sep 20, 2016

Consistently support graceful and immediate termination for all objects #1535

Closed

Deleting deployment does not delete pods associated with it #32985

Closed

bgrant0607 mentioned this issue Nov 18, 2016

Deleting a replication controller using the REST API doesn't delete the pods #10564

Closed

caesarxuchao mentioned this issue Dec 13, 2016

GC changes for synchronous GC supports #38679

Closed

3 tasks

caesarxuchao mentioned this issue Mar 10, 2017

Garbage collector should orphan RS created by deployment when deleteOptions.OrphanDependents is true #42639

Closed

enisoc mentioned this issue Mar 10, 2017

DaemonSet: Respect ControllerRef #42173

Merged

enisoc mentioned this issue Mar 11, 2017

GC: Fix re-adoption race when orphaning dependents. #42938

Merged

This was referenced Mar 14, 2017

TestBlockingOwnerRefDoesBlock {garbagecollector} #42952

Closed

Wait for garbagecollector to be synced in test #43110

Merged

caesarxuchao mentioned this issue Jul 5, 2017

allow a deletestrategy to opt-out of GC #48354

Merged

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 31, 2017

asymmetric mentioned this issue Jan 29, 2018

Use ownerReferences with CRD habitat-sh/habitat-operator#38

Closed

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 30, 2018

k8s-ci-robot closed this as completed Mar 1, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Garbage Collector] Umbrella: known issues of garbage collector #26120

[Garbage Collector] Umbrella: known issues of garbage collector #26120

caesarxuchao commented May 23, 2016 •

edited

Loading

hongchaodeng commented May 23, 2016 •

edited

Loading

caesarxuchao commented May 23, 2016

caesarxuchao commented Jul 20, 2016

smarterclayton commented Jul 21, 2016

smarterclayton commented Jul 21, 2016

caesarxuchao commented Jul 25, 2016 •

edited

Loading

smarterclayton commented Jul 26, 2016 via email

0xmichalis commented Mar 10, 2017

caesarxuchao commented Mar 10, 2017

fejta-bot commented Dec 31, 2017

fejta-bot commented Jan 30, 2018

fejta-bot commented Mar 1, 2018

[Garbage Collector] Umbrella: known issues of garbage collector #26120

[Garbage Collector] Umbrella: known issues of garbage collector #26120

Comments

caesarxuchao commented May 23, 2016 • edited Loading

hongchaodeng commented May 23, 2016 • edited Loading

caesarxuchao commented May 23, 2016

caesarxuchao commented Jul 20, 2016

smarterclayton commented Jul 21, 2016

smarterclayton commented Jul 21, 2016

caesarxuchao commented Jul 25, 2016 • edited Loading

smarterclayton commented Jul 26, 2016 via email

0xmichalis commented Mar 10, 2017

caesarxuchao commented Mar 10, 2017

fejta-bot commented Dec 31, 2017

fejta-bot commented Jan 30, 2018

fejta-bot commented Mar 1, 2018

caesarxuchao commented May 23, 2016 •

edited

Loading

hongchaodeng commented May 23, 2016 •

edited

Loading

caesarxuchao commented Jul 25, 2016 •

edited

Loading