Cleanup based on comments on http://go/vcl/12764
Change-Id: I0b70794a4d295f8b3ced6ccadb9aec1faf3243a4
diff --git a/services/syncbase/clock/types.go b/services/syncbase/clock/types.go
index 2084eca..652909b 100644
--- a/services/syncbase/clock/types.go
+++ b/services/syncbase/clock/types.go
@@ -4,12 +4,16 @@
package clock
+import (
+ "time"
+)
+
// This interface provides a wrapper over system clock to allow easy testing
// of VClock and other code that uses timestamps. Tests can implement a mock
// SystemClock and set it on VClock using SetSystemClock() method.
type SystemClock interface {
- // Now returns the current UTC time in nanoseconds as known by the system.
+ // Now returns the current UTC time as known by the system.
// This may not reflect the real UTC time if the system clock is out of
// sync with UTC.
- Now() int64
+ Now() time.Time
}
diff --git a/services/syncbase/clock/vclock.go b/services/syncbase/clock/vclock.go
index 1e9acf2..6da96e3 100644
--- a/services/syncbase/clock/vclock.go
+++ b/services/syncbase/clock/vclock.go
@@ -16,9 +16,9 @@
// - clock : Instance of clock.SystemClock interface providing access
// to the system time.
type VClock struct {
- systemTimeAtBoot int64
- utcTimeAtBoot int64
- skew int64
+ systemTimeAtBoot time.Time
+ utcTimeAtBoot time.Time
+ skew time.Duration
clock SystemClock
}
@@ -30,13 +30,13 @@
// Now returns current UTC time based on the estimation of skew that
// the system clock has with respect to NTP.
-func (c *VClock) Now() int64 {
+func (c *VClock) Now() time.Time {
// This method returns just the current system time for now.
// TODO(jlodhia): implement estimation of UTC time.
return c.clock.Now()
}
-// This method allows tests to set a mock clock instance for testability
+// This method allows tests to set a mock clock instance for testability.
func (c *VClock) SetSystemClock(sysClock SystemClock) {
c.clock = sysClock
}
@@ -46,8 +46,8 @@
type systemClockImpl struct{}
-func (sc *systemClockImpl) Now() int64 {
- return time.Now().UTC().UnixNano()
+func (sc *systemClockImpl) Now() time.Time {
+ return time.Now().UTC()
}
var _ SystemClock = (*systemClockImpl)(nil)
diff --git a/services/syncbase/clock/vclock_test.go b/services/syncbase/clock/vclock_test.go
index 477d142..4221feb 100644
--- a/services/syncbase/clock/vclock_test.go
+++ b/services/syncbase/clock/vclock_test.go
@@ -6,17 +6,18 @@
import (
"testing"
+ "time"
)
type systemClockMockImpl struct {
- now int64
+ now time.Time
}
-func (sc *systemClockMockImpl) Now() int64 {
+func (sc *systemClockMockImpl) Now() time.Time {
return sc.now
}
-func (sc *systemClockMockImpl) setNow(now int64) {
+func (sc *systemClockMockImpl) setNow(now time.Time) {
sc.now = now
}
@@ -27,7 +28,7 @@
func TestVClock(t *testing.T) {
clock := NewVClock()
sysClock := &systemClockMockImpl{}
- writeTs := int64(4)
+ writeTs := time.Now()
sysClock.setNow(writeTs)
clock.SetSystemClock(sysClock)
diff --git a/services/syncbase/server/watchable/store.go b/services/syncbase/server/watchable/store.go
index 23fc48f..2aaa997 100644
--- a/services/syncbase/server/watchable/store.go
+++ b/services/syncbase/server/watchable/store.go
@@ -72,8 +72,8 @@
}
var err error
seq, err = strconv.ParseUint(parts[1], 10, 64)
- // Current value of seq points to the last transaction committed
- // increment the value by 1.
+ // Current value of seq points to the last transaction committed.
+ // Increment this value by 1.
seq++
if err != nil {
panic("failed to parse seq: " + key)
@@ -87,7 +87,7 @@
ist store.Store
opts *Options
mu sync.Mutex // held during transaction commits; protects seq
- seq uint64 // sequence number, for commits
+ seq uint64 // the next sequence number to be used for a new commit
clock *clock.VClock // used to provide write timestamps
}
diff --git a/services/syncbase/server/watchable/store_test.go b/services/syncbase/server/watchable/store_test.go
index 8338342..5ae2562 100644
--- a/services/syncbase/server/watchable/store_test.go
+++ b/services/syncbase/server/watchable/store_test.go
@@ -9,7 +9,6 @@
"testing"
"v.io/syncbase/x/ref/services/syncbase/store"
- "v.io/syncbase/x/ref/services/syncbase/store/memstore"
"v.io/syncbase/x/ref/services/syncbase/store/test"
)
@@ -69,18 +68,11 @@
const useMemstore = false
func runTest(t *testing.T, mp []string, f func(t *testing.T, st store.Store)) {
- var st store.Store
- if useMemstore {
- st = memstore.New()
- } else {
- var dbPath string = getPath()
- st = createLevelDB(dbPath)
- defer destroyLevelDB(st, dbPath)
- }
+ st, destroy := createStore(useMemstore)
+ defer destroy()
st, err := Wrap(st, &Options{ManagedPrefixes: mp})
if err != nil {
t.Fatal(err)
}
- defer st.Close()
f(t, st)
}
diff --git a/services/syncbase/server/watchable/test_util.go b/services/syncbase/server/watchable/test_util.go
index c1877e5..5a5d6dc 100644
--- a/services/syncbase/server/watchable/test_util.go
+++ b/services/syncbase/server/watchable/test_util.go
@@ -7,6 +7,7 @@
import (
"fmt"
"io/ioutil"
+ "time"
"v.io/syncbase/x/ref/services/syncbase/clock"
"v.io/syncbase/x/ref/services/syncbase/store"
@@ -17,8 +18,10 @@
// This file provides utility methods for tests related to watchable store.
-//**** Functions related to creation/cleanup of store instances **//
+/////// Functions related to creation/cleanup of store instances ///////
+// createStore returns a store along with a function to destroy the store
+// once it is no longer needed.
func createStore(useMemstore bool) (store.Store, func()) {
var st store.Store
if useMemstore {
@@ -57,7 +60,7 @@
}
}
-//**** Functions related to watchable store **//
+/////// Functions related to watchable store ///////
func getSeq(st Store) uint64 {
wst := st.(*wstore)
@@ -91,24 +94,24 @@
return key, entry
}
-//*** Clock related utility code ********//
+/////// Clock related utility code ///////
// Mock Implementation for SystemClock
type MockSystemClock struct {
- time int64 // current time returned by call to Now()
- increment int64 // how much to increment the clock by for subsequent calls to Now()
+ time time.Time // current time returned by call to Now()
+ increment time.Duration // how much to increment the clock by for subsequent calls to Now()
}
-func NewMockSystemClock(firstTimestamp, increment int64) *MockSystemClock {
+func NewMockSystemClock(firstTimestamp time.Time, increment time.Duration) *MockSystemClock {
return &MockSystemClock{
time: firstTimestamp,
increment: increment,
}
}
-func (sc *MockSystemClock) Now() int64 {
+func (sc *MockSystemClock) Now() time.Time {
now := sc.time
- sc.time += sc.increment
+ sc.time = sc.time.Add(sc.increment)
return now
}
diff --git a/services/syncbase/server/watchable/transaction.go b/services/syncbase/server/watchable/transaction.go
index e3d8422..3cbff62 100644
--- a/services/syncbase/server/watchable/transaction.go
+++ b/services/syncbase/server/watchable/transaction.go
@@ -117,7 +117,7 @@
return verror.New(verror.ErrInternal, nil, "seq maxed out")
}
// Write LogEntry records.
- timestamp := tx.st.clock.Now()
+ timestamp := tx.st.clock.Now().UnixNano()
keyPrefix := getLogEntryKeyPrefix(tx.st.seq)
for txSeq, op := range tx.ops {
key := join(keyPrefix, fmt.Sprintf("%04x", txSeq))
@@ -150,7 +150,7 @@
return tx.itx.Abort()
}
-// Used for testing
+// Exported as a helper function for testing purposes
func getLogEntryKeyPrefix(seq uint64) string {
// Note, MaxUint16 is 0xffff and MaxUint64 is 0xffffffffffffffff.
// TODO(sadovsky): Use a more space-efficient lexicographic number encoding.
diff --git a/services/syncbase/server/watchable/transaction_test.go b/services/syncbase/server/watchable/transaction_test.go
index 6da6422..54405a2 100644
--- a/services/syncbase/server/watchable/transaction_test.go
+++ b/services/syncbase/server/watchable/transaction_test.go
@@ -8,6 +8,7 @@
"bytes"
"fmt"
"testing"
+ "time"
"v.io/syncbase/x/ref/services/syncbase/store"
)
@@ -36,10 +37,12 @@
updateVal: "val-b2",
}
-func TestLogEntityTimestamps(t *testing.T) {
+func TestLogEntryTimestamps(t *testing.T) {
stImpl, destroy := createStore(useMemstoreForTest)
defer destroy()
- var mockClock *MockSystemClock = NewMockSystemClock(3, 1)
+ t1 := time.Now()
+ inc := time.Duration(1) * time.Second
+ var mockClock *MockSystemClock = NewMockSystemClock(t1, inc)
wst1, err := Wrap(stImpl, &Options{ManagedPrefixes: nil})
if err != nil {
@@ -65,10 +68,10 @@
panic(fmt.Errorf("can't commit transaction: %v", err))
}
- // read and verify LogEntities written as part of above transaction
- // We expect 2 entires in the log for the two puts.
- // Timestamp from mockclock for the commit shoud be 3
- verifyCommitLog(t, stImpl, seqForCreate, 2, 3)
+ // read and verify LogEntries written as part of above transaction
+ // We expect 2 entries in the log for the two puts.
+ // Timestamp from mockclock for the commit should be t1
+ verifyCommitLog(t, stImpl, seqForCreate, 2, t1)
// Update data already present in store with a new watchable store
wst2, err := Wrap(stImpl, &Options{ManagedPrefixes: nil})
@@ -93,10 +96,11 @@
panic(fmt.Errorf("can't commit transaction: %v", err))
}
- // read and verify LogEntities written as part of above transaction
- // We expect 4 entires in the log for the two gets and two puts.
- // Timestamp from mockclock for the commit shoud be 4
- verifyCommitLog(t, stImpl, seqForUpdate, 4, 4)
+ // read and verify LogEntries written as part of above transaction
+ // We expect 4 entries in the log for the two gets and two puts.
+ // Timestamp from mockclock for the commit should be t1 + 1 sec
+ t2 := t1.Add(inc)
+ verifyCommitLog(t, stImpl, seqForUpdate, 4, t2)
}
func checkAndUpdate(st store.StoreReadWriter, data testData) error {
@@ -116,16 +120,16 @@
return nil
}
-func verifyCommitLog(t *testing.T, st store.Store, seq uint64, expectedEntries int, expectedTimestamp int64) {
+func verifyCommitLog(t *testing.T, st store.Store, seq uint64, expectedEntries int, expectedTimestamp time.Time) {
var ler *LogEntryReader = NewLogEntryReader(st, seq)
var entryCount int = 0
for ler.Advance() {
_, entry := ler.GetEntry()
entryCount++
- if entry.CommitTimestamp != expectedTimestamp {
- errStr := "Unexpected timestamp found for entity." +
+ if entry.CommitTimestamp != expectedTimestamp.UnixNano() {
+ errStr := "Unexpected timestamp found for entry." +
" Expected: %d, found: %d"
- t.Errorf(errStr, expectedTimestamp, entry.CommitTimestamp)
+ t.Errorf(errStr, expectedTimestamp.UnixNano(), entry.CommitTimestamp)
}
}
if entryCount != expectedEntries {
