From 3b12b5e85cc4e066e102757ed04d5f3b81e97950 Mon Sep 17 00:00:00 2001 From: paul cannon Date: Fri, 29 Mar 2019 15:07:07 -0600 Subject: [PATCH] non-racy CreateEntryIfNotExists() (#1611) This changes semantics slightly! with this change, CreateEntryIfNotExists() will do a cache Update with every node passed in, whether it exists or not. Update() already does a race-free upsert operation, so that change removes the problematic race in CreateEntryIfNotExists(). As far as I can tell, this semantic change doesn't break any expectations of callers, and shouldn't affect performance in a significant way, as we already have an awful lot of round-trips to the db either way. But if I've misunderstood the intention of the method, someone ought to catch it during review. --- satellite/satellitedb/overlaycache.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/satellite/satellitedb/overlaycache.go b/satellite/satellitedb/overlaycache.go index d544baea7..a9c708df0 100644 --- a/satellite/satellitedb/overlaycache.go +++ b/satellite/satellitedb/overlaycache.go @@ -563,16 +563,18 @@ func (cache *overlaycache) UpdateBatch(ctx context.Context, updateReqList []*ove func (cache *overlaycache) CreateEntryIfNotExists(ctx context.Context, node *pb.Node) (stats *overlay.NodeStats, err error) { defer mon.Task()(&ctx)(&err) - getStats, err := cache.GetStats(ctx, node.Id) - if overlay.ErrNodeNotFound.Has(err) { - err = cache.Update(ctx, node) - if err != nil { - return nil, Error.Wrap(err) - } - - return cache.GetStats(ctx, node.Id) + // Update already does a non-racy create-or-update, so we don't need a + // transaction here. Changes may occur between Update and Get_Node_By_Id, + // but that doesn't break any semantics here. + err = cache.Update(ctx, node) + if err != nil { + return nil, err } - return getStats, err + dbNode, err := cache.db.Get_Node_By_Id(ctx, dbx.Node_Id(node.Id.Bytes())) + if err != nil { + return nil, Error.Wrap(err) + } + return getNodeStats(node.Id, dbNode), nil } func convertDBNode(info *dbx.Node) (*pb.Node, error) {