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.
This commit is contained in:
paul cannon 2019-03-29 15:07:07 -06:00 committed by GitHub
parent 857edee485
commit 3b12b5e85c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -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) {