diff --git a/server/dirstore.go b/server/dirstore.go index 5a6af63a..663aabef 100644 --- a/server/dirstore.go +++ b/server/dirstore.go @@ -686,18 +686,15 @@ func (store *DirJWTStore) startExpiring(reCheck time.Duration, limit int64, evic now := time.Now() store.Lock() if pq.Len() > 0 { - if it := heap.Pop(pq).(*jwtItem); it.expiration <= now.Unix() { + if it := pq.heap[0]; it.expiration <= now.Unix() { path := store.pathForKey(it.publicKey) - if err := os.Remove(path); err != nil { - heap.Push(pq, it) // retry later - } else { + if err := os.Remove(path); err == nil { + heap.Pop(pq) pq.unTrack(it.publicKey) xorAssign(&pq.hash, it.hash) store.Unlock() - continue // we removed an entry, check next one + continue // we removed an entry, check next one right away } - } else { - heap.Push(pq, it) } } store.Unlock() diff --git a/server/dirstore_test.go b/server/dirstore_test.go index 1fd33783..58fd1406 100644 --- a/server/dirstore_test.go +++ b/server/dirstore_test.go @@ -22,6 +22,7 @@ import ( "fmt" "io/ioutil" "math" + "math/rand" "os" "path/filepath" "strings" @@ -35,49 +36,49 @@ import ( func require_True(t *testing.T, b bool) { t.Helper() if !b { - t.Errorf("require true, but got false") + t.Fatalf("require true, but got false") } } func require_False(t *testing.T, b bool) { t.Helper() if b { - t.Errorf("require no false, but got true") + t.Fatalf("require no false, but got true") } } func require_NoError(t *testing.T, err error) { t.Helper() if err != nil { - t.Errorf("require no error, but got: %v", err) + t.Fatalf("require no error, but got: %v", err) } } func require_Error(t *testing.T, err error) { t.Helper() if err == nil { - t.Errorf("require no error, but got: %v", err) + t.Fatalf("require error, but got none") } } func require_Equal(t *testing.T, a, b string) { t.Helper() if strings.Compare(a, b) != 0 { - t.Errorf("require equal, but got: %v != %v", a, b) + t.Fatalf("require equal, but got: %v != %v", a, b) } } func require_NotEqual(t *testing.T, a, b [32]byte) { t.Helper() if bytes.Equal(a[:], b[:]) { - t.Errorf("require not equal, but got: %v != %v", a, b) + t.Fatalf("require not equal, but got: %v != %v", a, b) } } func require_Len(t *testing.T, a, b int) { t.Helper() if a != b { - t.Errorf("require len, but got: %v != %v", a, b) + t.Fatalf("require len, but got: %v != %v", a, b) } } @@ -436,28 +437,27 @@ func TestExpiration(t *testing.T) { createTestAccount(t, dirStore, expSec, accountKey) } - h := dirStore.Hash() + hBegin := dirStore.Hash() + account(100) + hNoExp := dirStore.Hash() + require_NotEqual(t, hBegin, hNoExp) + account(1) + nh2 := dirStore.Hash() + require_NotEqual(t, hNoExp, nh2) + assertStoreSize(t, dirStore, 2) - for i := 1; i <= 3; i++ { - account(i) - nh := dirStore.Hash() - require_NotEqual(t, h, nh) - h = nh - } - time.Sleep(500 * time.Millisecond) - for i := 3; i > 0; i-- { + failAt := time.Now().Add(4 * time.Second) + for time.Now().Before(failAt) { + time.Sleep(100 * time.Millisecond) f, err := ioutil.ReadDir(dir) require_NoError(t, err) - require_Len(t, len(f), i) - assertStoreSize(t, dirStore, i) - - time.Sleep(time.Second) - - nh := dirStore.Hash() - require_NotEqual(t, h, nh) - h = nh + if len(f) == 1 { + lh := dirStore.Hash() + require_Equal(t, string(hNoExp[:]), string(lh[:])) + return + } } - assertStoreSize(t, dirStore, 0) + t.Errorf("Waited more than 4 seconds for the file with expiration 1 second to expire") } func TestLimit(t *testing.T) { @@ -586,6 +586,48 @@ func TestLruLoad(t *testing.T) { require_NoError(t, err) } +func TestLruVolume(t *testing.T) { + dir, err := ioutil.TempDir(os.TempDir(), "jwtstore_test") + require_NoError(t, err) + + dirStore, err := NewExpiringDirJWTStore(dir, false, false, NoDelete, time.Millisecond*50, 2, true, 0, nil) + require_NoError(t, err) + defer dirStore.Close() + replaceCnt := 500 // needs to be bigger than 2 due to loop unrolling + keys := make([]string, replaceCnt) + + key, err := nkeys.CreateAccount() + require_NoError(t, err) + keys[0], err = key.PublicKey() + require_NoError(t, err) + createTestAccount(t, dirStore, 10000, key) // not intended to expire + assertStoreSize(t, dirStore, 1) + + key, err = nkeys.CreateAccount() + require_NoError(t, err) + keys[1], err = key.PublicKey() + require_NoError(t, err) + createTestAccount(t, dirStore, 10000, key) + assertStoreSize(t, dirStore, 2) + + for i := 2; i < replaceCnt; i++ { + k, err := nkeys.CreateAccount() + require_NoError(t, err) + keys[i], err = k.PublicKey() + require_NoError(t, err) + + createTestAccount(t, dirStore, 10000+rand.Intn(10000), k) // not intended to expire + assertStoreSize(t, dirStore, 2) + _, err = os.Stat(fmt.Sprintf("%s/%s.jwt", dir, keys[i-2])) + require_Error(t, err) + require_True(t, os.IsNotExist(err)) + _, err = os.Stat(fmt.Sprintf("%s/%s.jwt", dir, keys[i-1])) + require_NoError(t, err) + _, err = os.Stat(fmt.Sprintf("%s/%s.jwt", dir, keys[i])) + require_NoError(t, err) + } +} + func TestLru(t *testing.T) { dir, err := ioutil.TempDir(os.TempDir(), "jwtstore_test") require_NoError(t, err) @@ -605,11 +647,11 @@ func TestLru(t *testing.T) { pKey3, err := accountKey3.PublicKey() require_NoError(t, err) - createTestAccount(t, dirStore, 10, accountKey1) + createTestAccount(t, dirStore, 1000, accountKey1) assertStoreSize(t, dirStore, 1) - createTestAccount(t, dirStore, 10, accountKey2) + createTestAccount(t, dirStore, 1000, accountKey2) assertStoreSize(t, dirStore, 2) - createTestAccount(t, dirStore, 10, accountKey3) + createTestAccount(t, dirStore, 1000, accountKey3) assertStoreSize(t, dirStore, 2) _, err = os.Stat(fmt.Sprintf("%s/%s.jwt", dir, pKey1)) require_Error(t, err) @@ -618,7 +660,7 @@ func TestLru(t *testing.T) { require_NoError(t, err) // update -> will change this keys position for eviction - createTestAccount(t, dirStore, 10, accountKey2) + createTestAccount(t, dirStore, 1000, accountKey2) assertStoreSize(t, dirStore, 2) // recreate -> will evict 3 createTestAccount(t, dirStore, 1, accountKey1) @@ -631,7 +673,7 @@ func TestLru(t *testing.T) { _, err = os.Stat(fmt.Sprintf("%s/%s.jwt", dir, pKey1)) require_True(t, os.IsNotExist(err)) // recreate key3 - no eviction - createTestAccount(t, dirStore, 10, accountKey3) + createTestAccount(t, dirStore, 1000, accountKey3) assertStoreSize(t, dirStore, 2) }