Switch type; add comments & disclaimer

This commit is contained in:
Daniel Smith 2015-10-02 15:09:54 -07:00
parent 4bdb1259a7
commit 57c6dd93ea

View File

@ -43,15 +43,18 @@ import (
// TODO: consider merging keyLister with this object, tracking a list of // TODO: consider merging keyLister with this object, tracking a list of
// "known" keys when Pop() is called. Have to think about how that // "known" keys when Pop() is called. Have to think about how that
// affects error retrying. // affects error retrying.
// TODO(lavalamp): I believe there is a possible race only when using an
// external known object source that the above TODO would
// fix.
// //
// Also see the comment on DeltaFIFO. // Also see the comment on DeltaFIFO.
func NewDeltaFIFO(keyFunc KeyFunc, compressor DeltaCompressor, knownObjectKeys KeyLister) *DeltaFIFO { func NewDeltaFIFO(keyFunc KeyFunc, compressor DeltaCompressor, knownObjects KeyListerGetter) *DeltaFIFO {
f := &DeltaFIFO{ f := &DeltaFIFO{
items: map[string]Deltas{}, items: map[string]Deltas{},
queue: []string{}, queue: []string{},
keyFunc: keyFunc, keyFunc: keyFunc,
deltaCompressor: compressor, deltaCompressor: compressor,
knownObjectKeys: knownObjectKeys, knownObjects: knownObjects,
} }
f.cond.L = &f.lock f.cond.L = &f.lock
return f return f
@ -80,9 +83,8 @@ func NewDeltaFIFO(keyFunc KeyFunc, compressor DeltaCompressor, knownObjectKeys K
// //
// A note on the KeyLister used by the DeltaFIFO: It's main purpose is // A note on the KeyLister used by the DeltaFIFO: It's main purpose is
// to list keys that are "known", for the puspose of figuring out which // to list keys that are "known", for the puspose of figuring out which
// items have been deleted when Replace() is called. If the given KeyLister // items have been deleted when Replace() or Delete() are called. The deleted
// also satisfies the KeyGetter interface, the deleted objet will be // objet will be included in the DeleteFinalStateUnknown markers. These objects
// included in the DeleteFinalStateUnknown markers. These objects
// could be stale. // could be stale.
// //
// You may provide a function to compress deltas (e.g., represent a // You may provide a function to compress deltas (e.g., represent a
@ -106,10 +108,10 @@ type DeltaFIFO struct {
// deltas. It may be nil. // deltas. It may be nil.
deltaCompressor DeltaCompressor deltaCompressor DeltaCompressor
// knownObjectKeys list keys that are "known", for the // knownObjects list keys that are "known", for the
// purpose of figuring out which items have been deleted // purpose of figuring out which items have been deleted
// when Replace() is called. // when Replace() or Delete() is called.
knownObjectKeys KeyLister knownObjects KeyListerGetter
} }
var ( var (
@ -164,19 +166,18 @@ func (f *DeltaFIFO) Delete(obj interface{}) error {
} }
f.lock.Lock() f.lock.Lock()
defer f.lock.Unlock() defer f.lock.Unlock()
if f.knownObjectKeys == nil { if f.knownObjects == nil {
if _, exists := f.items[id]; !exists { if _, exists := f.items[id]; !exists {
// Presumably, this was deleted when a relist happened. // Presumably, this was deleted when a relist happened.
// Don't provide a second report of the same deletion. // Don't provide a second report of the same deletion.
return nil return nil
} }
} else if keyGetter, ok := f.knownObjectKeys.(KeyGetter); ok { } else if _, exists, err := f.knownObjects.GetByKey(id); err == nil && !exists {
if _, exists, err := keyGetter.GetByKey(id); err == nil && !exists { // Presumably, this was deleted when a relist happened.
// Presumably, this was deleted when a relist happened. // Don't provide a second report of the same deletion.
// Don't provide a second report of the same deletion. // TODO(lavalamp): This may be racy-- we aren't properly locked
// This may be racy-- we aren't properly locked with knownObjectKeys. // with knownObjects.
return nil return nil
}
} }
return f.queueActionLocked(Deleted, obj) return f.queueActionLocked(Deleted, obj)
@ -228,7 +229,9 @@ func dedupDeltas(deltas Deltas) Deltas {
return deltas return deltas
} }
// If a & b represent the same event, returns the delta that ought to be kept. Otherwise, nil. // If a & b represent the same event, returns the delta that ought to be kept.
// Otherwise, returns nil.
// TODO: is there anything other than deletions that need deduping?
func isDup(a, b *Delta) *Delta { func isDup(a, b *Delta) *Delta {
if out := isDeletionDup(a, b); out != nil { if out := isDeletionDup(a, b); out != nil {
return out return out
@ -250,7 +253,7 @@ func isDeletionDup(a, b *Delta) *Delta {
} }
// queueActionLocked appends to the delta list for the object, calling // queueActionLocked appends to the delta list for the object, calling
// f.deltaCompressor if needed // f.deltaCompressor if needed. Caller must lock first.
func (f *DeltaFIFO) queueActionLocked(actionType DeltaType, obj interface{}) error { func (f *DeltaFIFO) queueActionLocked(actionType DeltaType, obj interface{}) error {
id, err := f.KeyOf(obj) id, err := f.KeyOf(obj)
if err != nil { if err != nil {
@ -381,7 +384,7 @@ func (f *DeltaFIFO) Replace(list []interface{}, resourceVersion string) error {
} }
} }
if f.knownObjectKeys == nil { if f.knownObjects == nil {
// Do deletion detection against our own list. // Do deletion detection against our own list.
for k, oldItem := range f.items { for k, oldItem := range f.items {
if keys.Has(k) { if keys.Has(k) {
@ -399,25 +402,21 @@ func (f *DeltaFIFO) Replace(list []interface{}, resourceVersion string) error {
} }
// Detect deletions not already in the queue. // Detect deletions not already in the queue.
knownKeys := f.knownObjectKeys.ListKeys() // TODO(lavalamp): This may be racy-- we aren't properly locked
// with knownObjects. Unproven.
knownKeys := f.knownObjects.ListKeys()
for _, k := range knownKeys { for _, k := range knownKeys {
if keys.Has(k) { if keys.Has(k) {
continue continue
} }
var deletedObj interface{} deletedObj, exists, err := f.knownObjects.GetByKey(k)
if keyGetter, ok := f.knownObjectKeys.(KeyGetter); ok { if err != nil {
var exists bool deletedObj = nil
var err error glog.Errorf("Unexpected error %v during lookup of key %v, placing DeleteFinalStateUnknown marker without object", err, k)
deletedObj, exists, err = keyGetter.GetByKey(k) } else if !exists {
if err != nil || !exists { deletedObj = nil
deletedObj = nil glog.Infof("Key %v does not exist in known objects store, placing DeleteFinalStateUnknown marker without object", k)
if err != nil {
glog.Errorf("Unexpected error %v during lookup of key %v, placing DeleteFinalStateUnknown marker without object", err, k)
} else {
glog.Infof("Key %v does not exist in known objects store, placing DeleteFinalStateUnknown marker without object", k)
}
}
} }
if err := f.queueActionLocked(Deleted, DeletedFinalStateUnknown{k, deletedObj}); err != nil { if err := f.queueActionLocked(Deleted, DeletedFinalStateUnknown{k, deletedObj}); err != nil {
return err return err