Skip to content

Commit

Permalink
CBG-4474 Fix for MV invalidation ordering (#7306)
Browse files Browse the repository at this point in the history 
Ensure that MV is invalidated before cv is moved to pv.

Created more extensive testing for AddVersion, and refactored AddVersion to reduce complexity based on the additional test coverage.
  • Loading branch information
@adamcfraser
adamcfraser authored Jan 18, 2025
1 parent 8d84ecc commit 8e9dc71
Show file tree
Hide file tree
Showing 2 changed files with 170 additions and 29 deletions.
46 changes: 17 additions & 29 deletions db/hybrid_logical_vector.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,52 +230,40 @@ func (hlv *HybridLogicalVector) DominatesSource(version Version) bool {

// AddVersion adds newVersion as the current version to the in memory representation of the HLV.
func (hlv *HybridLogicalVector) AddVersion(newVersion Version) error {
var newVersionCAS uint64
hlvVersionCAS := hlv.Version
if newVersion.Value != expandMacroCASValueUint64 {
newVersionCAS = newVersion.Value
}

// check if this is the first time we're adding a source - version pair
if hlv.SourceID == "" {
hlv.Version = newVersion.Value
hlv.SourceID = newVersion.SourceID
hlv.InvalidateMV()
return nil
}
// if new entry has the same source we simple just update the version

// If the new version is older than an existing version for the same source, return error
existingValueForSource, found := hlv.GetValue(newVersion.SourceID)
if found && existingValueForSource > newVersion.Value {
return fmt.Errorf("attempting to add new version vector entry with a value that is less than the existing value for the same source. New version: %v, Existing HLV: %v", newVersion, hlv)
}

// Move existing mv to pv before adding the new version
hlv.InvalidateMV()

// If the new version has the same source as existing cv, we just update the cv value
if newVersion.SourceID == hlv.SourceID {
if newVersion.Value != expandMacroCASValueUint64 && newVersionCAS < hlvVersionCAS {
return fmt.Errorf("attempting to add new version vector entry with a CAS that is less than the current version CAS value for the same source. Current cas: %d new cas %d", hlv.Version, newVersion.Value)
}
hlv.Version = newVersion.Value
hlv.InvalidateMV()
return nil
}
// if we get here this is a new version from a different sourceID thus need to move current sourceID to previous versions and update current version

// If we get here this is a new version from a different sourceID. Need to move existing cv to pv and update cv
if hlv.PreviousVersions == nil {
hlv.PreviousVersions = make(HLVVersions)
}
// we need to check if source ID already exists in PV, if so we need to ensure we are only updating with the
// sourceID-version pair if incoming version is greater than version already there
if currPVVersion, ok := hlv.PreviousVersions[hlv.SourceID]; ok {
// if we get here source ID exists in PV, only replace version if it is less than the incoming version
currPVVersionCAS := currPVVersion
if currPVVersionCAS < hlvVersionCAS {
hlv.PreviousVersions[hlv.SourceID] = hlv.Version
} else {
return fmt.Errorf("local hlv has current source in previous version with version greater than current version. Current CAS: %d, PV CAS %d", hlv.Version, currPVVersion)
}
} else {
// source doesn't exist in PV so add
hlv.PreviousVersions[hlv.SourceID] = hlv.Version
}

// If new version already exists PV, need to remove it
delete(hlv.PreviousVersions, newVersion.SourceID)
hlv.PreviousVersions[hlv.SourceID] = hlv.Version

// If new version source already existed in PV, need to remove it
delete(hlv.PreviousVersions, newVersion.SourceID)
hlv.Version = newVersion.Value
hlv.SourceID = newVersion.SourceID
hlv.InvalidateMV()
return nil
}

Expand Down
153 changes: 153 additions & 0 deletions db/hybrid_logical_vector_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1260,3 +1260,156 @@ func TestInvalidateMV(t *testing.T) {
})
}
}

func TestAddVersion(t *testing.T) {
testCases := []struct {
name string
initialHLV string
newVersion string
expectedHLV string
}{
{
name: "cv only, same source",
initialHLV: "100@a",
newVersion: "110@a",
expectedHLV: "110@a",
},
{
name: "cv only, different source",
initialHLV: "100@a",
newVersion: "110@b",
expectedHLV: "110@b;100@a",
},
{
name: "single pv, cv source update",
initialHLV: "100@b;90@a",
newVersion: "110@b",
expectedHLV: "110@b;90@a",
},
{
name: "single pv, pv source update",
initialHLV: "100@b;90@a",
newVersion: "110@a",
expectedHLV: "110@a;100@b",
},
{
name: "single pv, new source update",
initialHLV: "100@b;90@a",
newVersion: "110@c",
expectedHLV: "110@c;90@a,100@b",
},
{
name: "multiple pv, cv source update",
initialHLV: "100@b;90@a,80@c",
newVersion: "110@b",
expectedHLV: "110@b;90@a,80@c",
},
{
name: "multiple pv, pv source update",
initialHLV: "100@b;90@a,80@c",
newVersion: "110@a",
expectedHLV: "110@a;100@b,80@c",
},
{
name: "multiple pv, new source update",
initialHLV: "100@b;90@a,80@c",
newVersion: "110@d",
expectedHLV: "110@d;90@a,100@b,80@c",
},
{
name: "mv only, cv source update",
initialHLV: "100@b,90@b,80@a",
newVersion: "110@b",
expectedHLV: "110@b;80@a",
},
{
name: "mv only, mv source update",
initialHLV: "100@b,90@b,80@a",
newVersion: "110@a",
expectedHLV: "110@a;100@b",
},
{
name: "mv only, cv not in mv, cv source update",
initialHLV: "100@c,90@b,80@a",
newVersion: "110@c",
expectedHLV: "110@c;90@b,80@a",
},
{
name: "mv only, cv not in mv, mv source update",
initialHLV: "100@c,90@b,80@a",
newVersion: "110@b",
expectedHLV: "110@b;100@c,80@a",
},
{
name: "mv only, cv not in mv, new source update",
initialHLV: "100@c,90@b,80@a",
newVersion: "110@d",
expectedHLV: "110@d;100@c,90@b,80@a",
},
{
name: "mv and pv, cv source update",
initialHLV: "100@c,90@b,80@a;70@d",
newVersion: "110@c",
expectedHLV: "110@c;90@b,80@a,70@d",
},
{
name: "mv and pv, mv source update",
initialHLV: "100@c,90@b,80@a;70@d",
newVersion: "110@b",
expectedHLV: "110@b;100@c,80@a,70@d",
},
{
name: "mv and pv, pv source update",
initialHLV: "100@c,90@b,80@a;70@d",
newVersion: "110@d",
expectedHLV: "110@d;100@c,90@b,80@a",
},
{
name: "mv and pv, new source update",
initialHLV: "100@c,90@b,80@a;70@d",
newVersion: "110@e",
expectedHLV: "110@e;100@c,90@b,80@a,70@d",
},
{
name: "mv (with cv source) and pv, cv source update",
initialHLV: "100@b,90@b,80@a;70@c",
newVersion: "110@b",
expectedHLV: "110@b;80@a,70@c",
},
{
name: "mv (with cv source) and pv, mv source update",
initialHLV: "100@b,90@b,80@a;70@c",
newVersion: "110@a",
expectedHLV: "110@a;100@b,70@c",
},
{
name: "mv (with cv source) and pv, pv source update",
initialHLV: "100@b,90@b,80@a;70@c",
newVersion: "110@c",
expectedHLV: "110@c;100@b,80@a",
},
{
name: "mv (with cv source) and pv, new source update",
initialHLV: "100@b,90@b,80@a;70@c",
newVersion: "110@d",
expectedHLV: "110@d;100@b,80@a,70@c",
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {

hlv, _, err := ExtractHLVFromBlipMessage(tc.initialHLV)
require.NoError(t, err, "unable to parse initialHLV")
newVersion, err := ParseVersion(tc.newVersion)
require.NoError(t, err)

err = hlv.AddVersion(newVersion)
require.NoError(t, err)

expectedHLV, _, err := ExtractHLVFromBlipMessage(tc.expectedHLV)
require.NoError(t, err)
require.True(t, hlv.Equals(expectedHLV), "expected %#v does not match actual %#v", expectedHLV, hlv)

})
}
}

0 comments on commit 8e9dc71

Please sign in to comment.