Skip to content

Commit 395f2c9

Browse files
josephschorrjosephschorr
authored
Merge pull request authzed#1724 from josephschorr/e2e-flakes
HLC Parsing fixes
2 parents 3edd286 + 349eb20 commit 395f2c9

File tree

6 files changed

+59
-18
lines changed

6 files changed

+59
-18
lines changed

‎e2e/newenemy/newenemy_test.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,7 @@ func checkDataNoNewEnemy(ctx context.Context, t testing.TB, slowNodeID int, crdb
360360
Updates: []*v1.RelationshipUpdate{blockusers[i]},
361361
})
362362
require.NoError(t, err)
363+
t.Log("r1 token: ", r1.WrittenAt.Token)
363364

364365
// sleeping in between the writes seems to let cockroach clear out
365366
// pending transactions in the background and has a better chance
@@ -373,6 +374,13 @@ func checkDataNoNewEnemy(ctx context.Context, t testing.TB, slowNodeID int, crdb
373374
Updates: []*v1.RelationshipUpdate{allowlists[i]},
374375
})
375376
require.NoError(t, err)
377+
t.Log("r2 token: ", r2.WrittenAt.Token)
378+
379+
z1, _ := zedtoken.DecodeRevision(r1.WrittenAt, revisions.CommonDecoder{Kind: revisions.HybridLogicalClock})
380+
z2, _ := zedtoken.DecodeRevision(r2.WrittenAt, revisions.CommonDecoder{Kind: revisions.HybridLogicalClock})
381+
382+
t.Log("z1 revision: ", z1)
383+
t.Log("z2 revision: ", z2)
376384

377385
canHas, err := spicedb[1].Client().V1().Permissions().CheckPermission(context.Background(), &v1.CheckPermissionRequest{
378386
Consistency: &v1.Consistency{Requirement: &v1.Consistency_AtExactSnapshot{AtExactSnapshot: r2.WrittenAt}},
@@ -396,8 +404,6 @@ func checkDataNoNewEnemy(ctx context.Context, t testing.TB, slowNodeID int, crdb
396404
ns2AllowlistLeader := getLeaderNodeForNamespace(ctx, crdb[2].Conn(), allowlists[i].Relationship.Subject.Object.ObjectType)
397405

398406
r1leader, r2leader := getLeaderNode(ctx, crdb[2].Conn(), blockusers[i].Relationship), getLeaderNode(ctx, crdb[2].Conn(), allowlists[i].Relationship)
399-
z1, _ := zedtoken.DecodeRevision(r1.WrittenAt, revisions.CommonDecoder{Kind: revisions.HybridLogicalClock})
400-
z2, _ := zedtoken.DecodeRevision(r2.WrittenAt, revisions.CommonDecoder{Kind: revisions.HybridLogicalClock})
401407
t.Log(sleep, z1, z2, z1.GreaterThan(z2), r1leader, r2leader, ns1BlocklistLeader, ns1UserLeader, ns2ResourceLeader, ns2AllowlistLeader)
402408

403409
if z1.GreaterThan(z2) {

‎internal/datastore/crdb/crdb.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,7 @@ func readCRDBNow(ctx context.Context, reader pgxcommon.DBFuncQuerier) (datastore
478478
return datastore.NoRevision, fmt.Errorf("unable to read timestamp: %w", err)
479479
}
480480

481-
return revisions.NewForHLC(hlcNow), nil
481+
return revisions.NewForHLC(hlcNow)
482482
}
483483

484484
func readClusterTTLNanos(ctx context.Context, conn pgxcommon.DBFuncQuerier) (int64, error) {

‎internal/datastore/crdb/stats.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,5 +105,5 @@ func updateCounter(ctx context.Context, tx pgx.Tx, change int64) (datastore.Revi
105105
return datastore.NoRevision, fmt.Errorf("unable to executed upsert counter query: %w", err)
106106
}
107107

108-
return revisions.NewForHLC(timestamp), nil
108+
return revisions.NewForHLC(timestamp)
109109
}

‎internal/datastore/revisions/hlcrevision.go

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,14 @@ func parseHLCRevisionString(revisionStr string) (datastore.Revision, error) {
5050
return datastore.NoRevision, fmt.Errorf("invalid revision string: %q", revisionStr)
5151
}
5252

53-
logicalclock, err := strconv.ParseInt(pieces[1], 10, 32)
54-
if err != nil {
55-
return datastore.NoRevision, fmt.Errorf("invalid revision string: %q", revisionStr)
53+
if len(pieces[1]) > logicalClockLength {
54+
return datastore.NoRevision, spiceerrors.MustBugf("invalid revision string due to unexpected logical clock size (%d): %q", len(pieces[1]), revisionStr)
5655
}
5756

58-
if len(pieces[1]) != logicalClockLength {
59-
return datastore.NoRevision, spiceerrors.MustBugf("invalid revision string due to unexpected logical clock size (%d): %q", len(pieces[1]), revisionStr)
57+
paddedLogicalClockStr := pieces[1] + strings.Repeat("0", logicalClockLength-len(pieces[1]))
58+
logicalclock, err := strconv.ParseInt(paddedLogicalClockStr, 10, 32)
59+
if err != nil {
60+
return datastore.NoRevision, fmt.Errorf("invalid revision string: %q", revisionStr)
6061
}
6162

6263
return HLCRevision{timestamp, uint32(logicalclock) + logicalClockOffset}, nil
@@ -73,9 +74,13 @@ func HLCRevisionFromString(revisionStr string) (HLCRevision, error) {
7374
}
7475

7576
// NewForHLC creates a new revision for the given hybrid logical clock.
76-
func NewForHLC(decimal decimal.Decimal) HLCRevision {
77-
rev, _ := HLCRevisionFromString(decimal.String())
78-
return rev
77+
func NewForHLC(decimal decimal.Decimal) (HLCRevision, error) {
78+
rev, err := HLCRevisionFromString(decimal.String())
79+
if err != nil {
80+
return zeroHLC, fmt.Errorf("invalid HLC decimal: %v (%s) => %w", decimal, decimal.String(), err)
81+
}
82+
83+
return rev, nil
7984
}
8085

8186
// NewHLCForTime creates a new revision for the given time.

‎internal/datastore/revisions/hlcrevision_test.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,17 @@ func TestNewForHLC(t *testing.T) {
1717
"-1",
1818
"1.0000000023",
1919
"1703283409994227985.0000000004",
20+
"1703283409994227985.0000000040",
21+
"1703283409994227985.0010000000",
2022
}
2123

2224
for _, tc := range tcs {
2325
t.Run(tc, func(t *testing.T) {
2426
d, err := decimal.NewFromString(tc)
2527
require.NoError(t, err)
2628

27-
rev := NewForHLC(d)
29+
rev, err := NewForHLC(d)
30+
require.NoError(t, err)
2831
require.Equal(t, tc, rev.String())
2932
})
3033
}
@@ -40,6 +43,7 @@ func TestTimestampNanoSec(t *testing.T) {
4043
"1.0000000023": 1,
4144
"9223372036854775807.0000000002": 9223372036854775807,
4245
"1703283409994227985.0000000004": 1703283409994227985,
46+
"1703283409994227985.0000000040": 1703283409994227985,
4347
}
4448

4549
for tc, nano := range tcs {
@@ -62,6 +66,11 @@ func TestInexactFloat64(t *testing.T) {
6266
"1.0000000023": 1.0000000023,
6367
"9223372036854775807.0000000002": 9223372036854775807.0000000002,
6468
"1703283409994227985.0000000004": 1703283409994227985.0000000004,
69+
"1703283409994227985.0000000040": 1703283409994227985.000000004,
70+
"1703283409994227985.000000004": 1703283409994227985.000000004,
71+
"1703283409994227985.0010": 1703283409994227985.001,
72+
"1703283409994227985.0010000000": 1703283409994227985.001,
73+
"1703283409994227985.001": 1703283409994227985.001,
6574
}
6675

6776
for tc, floatValue := range tcs {
@@ -116,6 +125,15 @@ func TestHLCKeyEquals(t *testing.T) {
116125
{
117126
"1703283409994227985.0000000005", "1703283409994227985.0000000005", true,
118127
},
128+
{
129+
"1703283409994227985.0000000050", "1703283409994227985.0000000050", true,
130+
},
131+
{
132+
"1703283409994227985.0000000050", "1703283409994227985.0000000005", false,
133+
},
134+
{
135+
"1703283409994227985.000000005", "1703283409994227985.0000000050", true,
136+
},
119137
}
120138

121139
for _, tc := range tcs {

‎pkg/zedtoken/zedtoken_test.go

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -166,10 +166,16 @@ var hlcDecodeTests = []struct {
166166
expectError bool
167167
}{
168168
{
169-
format: "V1 ZedToken",
170-
token: "CAIaFQoTMTYyMTUzODE4OTAyODkyODAwMA==",
171-
expectedRevision: revisions.NewForHLC(decimal.NewFromInt(1621538189028928000)),
172-
expectError: false,
169+
format: "V1 ZedToken",
170+
token: "CAIaFQoTMTYyMTUzODE4OTAyODkyODAwMA==",
171+
expectedRevision: func() datastore.Revision {
172+
r, err := revisions.NewForHLC(decimal.NewFromInt(1621538189028928000))
173+
if err != nil {
174+
panic(err)
175+
}
176+
return r
177+
}(),
178+
expectError: false,
173179
},
174180
{
175181
format: "V1 ZedToken",
@@ -179,7 +185,13 @@ var hlcDecodeTests = []struct {
179185
if err != nil {
180186
panic(err)
181187
}
182-
return revisions.NewForHLC(v)
188+
189+
r, err := revisions.NewForHLC(v)
190+
if err != nil {
191+
panic(err)
192+
}
193+
194+
return r
183195
})(),
184196
expectError: false,
185197
},

0 commit comments

Comments
 (0)