Skip to content

Commit dccad8c

Browse files
authored
Fix return wrong TTL when pumping job from the secondary storage (#218)
Currently, we put now() + TTL as the expired time when storing jobs in the database, but didn't remove the now() after pumping jobs. This PR also removes the unused function `BatchGetJobs` which is similar to GetReadyJobs.
1 parent 6122a4b commit dccad8c

File tree

5 files changed

+115
-181
lines changed

5 files changed

+115
-181
lines changed

storage/persistence/model/job.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,13 @@ type DBJob struct {
1212
CreatedTime int64 `spanner:"created_time" json:"created_time"`
1313
}
1414

15+
func (j *DBJob) TTL(now int64) int64 {
16+
if j.ExpiredTime == 0 {
17+
return 0
18+
}
19+
return j.ExpiredTime - now
20+
}
21+
1522
type DBJobReq struct {
1623
PoolName string
1724
Namespace string

storage/persistence/spanner/setup_test.go

Lines changed: 11 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ package spanner
33
import (
44
"context"
55
"fmt"
6-
"io/ioutil"
6+
"os"
77
"strings"
8-
"time"
8+
"testing"
99

1010
"cloud.google.com/go/spanner"
1111
database "cloud.google.com/go/spanner/admin/database/apiv1"
@@ -15,14 +15,6 @@ import (
1515
"google.golang.org/grpc/codes"
1616

1717
"github.com/bitleak/lmstfy/config"
18-
"github.com/bitleak/lmstfy/engine"
19-
"github.com/bitleak/lmstfy/storage/persistence/model"
20-
)
21-
22-
var (
23-
poolName = "default"
24-
jobIDs = []string{"1", "2", "3"}
25-
ctx = context.Background()
2618
)
2719

2820
func CreateInstance(ctx context.Context, cfg *config.SpannerConfig) error {
@@ -67,7 +59,7 @@ func CreateDatabase(ctx context.Context, cfg *config.SpannerConfig) error {
6759
return nil
6860
}
6961

70-
ddlBytes, err := ioutil.ReadFile("../../../scripts/schemas/spanner/ddls.sql")
62+
ddlBytes, err := os.ReadFile("../../../scripts/schemas/spanner/ddls.sql")
7163
if err != nil {
7264
return fmt.Errorf("read ddls file: %w", err)
7365
}
@@ -90,40 +82,15 @@ func CreateDatabase(ctx context.Context, cfg *config.SpannerConfig) error {
9082
return err
9183
}
9284

93-
func createTestJobsData() []engine.Job {
94-
jobs := make([]engine.Job, 0)
95-
j1 := engine.NewJob("n1", "q1", []byte("hello_j1"), 120, 30, 1, "1")
96-
j2 := engine.NewJob("n1", "q2", []byte("hello_j2"), 120, 60, 1, "2")
97-
j3 := engine.NewJob("n1", "q1", []byte("hello_j3"), 120, 90, 1, "3")
98-
jobs = append(jobs, j1, j2, j3)
99-
return jobs
100-
}
101-
102-
func createTestReqData() []*model.DBJobReq {
103-
req := make([]*model.DBJobReq, 0)
104-
r1 := &model.DBJobReq{
105-
PoolName: poolName,
106-
Namespace: "n1",
107-
Queue: "q1",
108-
ReadyTime: 0,
109-
Count: 10,
85+
func TestMain(m *testing.M) {
86+
if os.Getenv("SPANNER_EMULATOR_HOST") == "" {
87+
panic("SPANNER_EMULATOR_HOST is not set")
11088
}
111-
r2 := &model.DBJobReq{
112-
PoolName: poolName,
113-
Namespace: "n1",
114-
Queue: "q2",
115-
ReadyTime: 0,
116-
Count: 10,
89+
if err := CreateInstance(context.Background(), config.SpannerEmulator); err != nil {
90+
panic("Create instance: " + err.Error())
11791
}
118-
req = append(req, r1, r2)
119-
return req
120-
}
121-
122-
func createTestReqData2() *model.DBJobReq {
123-
req := &model.DBJobReq{
124-
PoolName: poolName,
125-
ReadyTime: time.Now().Unix() + 80,
126-
Count: 10,
92+
if err := CreateDatabase(context.Background(), config.SpannerEmulator); err != nil {
93+
panic("Create database: " + err.Error())
12794
}
128-
return req
95+
os.Exit(m.Run())
12996
}

storage/persistence/spanner/spanner.go

Lines changed: 14 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,17 @@ func (s *Spanner) BatchAddJobs(ctx context.Context, poolName string, jobs []engi
5454
now := time.Now().Unix()
5555
dbJobs := make([]*model.DBJob, 0)
5656
for _, job := range jobs {
57+
expiredTime := int64(0)
58+
if job.TTL() > 0 {
59+
expiredTime = now + int64(job.TTL())
60+
}
5761
j := &model.DBJob{
5862
PoolName: poolName,
5963
JobID: job.ID(),
6064
Namespace: job.Namespace(),
6165
Queue: job.Queue(),
6266
Body: job.Body(),
63-
ExpiredTime: now + int64(job.TTL()),
67+
ExpiredTime: expiredTime,
6468
ReadyTime: now + int64(job.Delay()),
6569
Tries: int64(job.Tries()),
6670
CreatedTime: now,
@@ -82,42 +86,6 @@ func (s *Spanner) BatchAddJobs(ctx context.Context, poolName string, jobs []engi
8286
return err
8387
}
8488

85-
// BatchGetJobs pumps data that are due before certain due time
86-
func (s *Spanner) BatchGetJobs(ctx context.Context, req []*model.DBJobReq) (jobs []engine.Job, err error) {
87-
txn := s.cli.ReadOnlyTransaction()
88-
now := time.Now().Unix()
89-
defer txn.Close()
90-
91-
for _, r := range req {
92-
iter := txn.Query(ctx, spanner.Statement{
93-
SQL: "SELECT pool_name, job_id, namespace, queue, body, ready_time, expired_time, created_time, tries " +
94-
"FROM lmstfy_jobs WHERE pool_name = @poolname and namespace = @namespace and queue = @queue and ready_time >= @readytime LIMIT @limit",
95-
Params: map[string]interface{}{
96-
"poolname": r.PoolName,
97-
"namespace": r.Namespace,
98-
"queue": r.Queue,
99-
"readytime": r.ReadyTime,
100-
"limit": r.Count,
101-
},
102-
})
103-
err = iter.Do(func(row *spanner.Row) error {
104-
elem := &model.DBJob{}
105-
if err = row.ToStruct(elem); err != nil {
106-
return err
107-
}
108-
j := engine.NewJob(elem.Namespace, elem.Queue, elem.Body, uint32(elem.ExpiredTime),
109-
uint32(elem.ReadyTime-now), uint16(elem.Tries), elem.JobID)
110-
jobs = append(jobs, j)
111-
return nil
112-
})
113-
114-
if err != nil {
115-
return nil, err
116-
}
117-
}
118-
return jobs, nil
119-
}
120-
12189
// GetQueueSize returns the size of data in storage which are due before certain due time
12290
func (s *Spanner) GetQueueSize(ctx context.Context, req []*model.DBJobReq) (count map[string]int64, err error) {
12391
txn := s.cli.ReadOnlyTransaction()
@@ -180,13 +148,14 @@ func (s *Spanner) GetReadyJobs(ctx context.Context, req *model.DBJobReq) (jobs [
180148
"limit": req.Count,
181149
},
182150
})
151+
183152
err = iter.Do(func(row *spanner.Row) error {
184-
elem := &model.DBJob{}
185-
if err = row.ToStruct(elem); err != nil {
153+
dbJob := &model.DBJob{}
154+
if err = row.ToStruct(dbJob); err != nil {
186155
return err
187156
}
188-
j := engine.NewJob(elem.Namespace, elem.Queue, elem.Body, uint32(elem.ExpiredTime),
189-
uint32(elem.ReadyTime-now), uint16(elem.Tries), elem.JobID)
157+
j := engine.NewJob(dbJob.Namespace, dbJob.Queue, dbJob.Body, uint32(dbJob.TTL(now)),
158+
uint32(dbJob.ReadyTime-now), uint16(dbJob.Tries), dbJob.JobID)
190159
jobs = append(jobs, j)
191160
return nil
192161
})
@@ -211,12 +180,12 @@ func (s *Spanner) BatchGetJobsByID(ctx context.Context, IDs []string) (jobs []en
211180
},
212181
})
213182
err = iter.Do(func(row *spanner.Row) error {
214-
elem := &model.DBJob{}
215-
if err = row.ToStruct(elem); err != nil {
183+
dbJob := &model.DBJob{}
184+
if err = row.ToStruct(dbJob); err != nil {
216185
return err
217186
}
218-
j := engine.NewJob(elem.Namespace, elem.Queue, elem.Body, uint32(elem.ExpiredTime),
219-
uint32(elem.ReadyTime-now), uint16(elem.Tries), elem.JobID)
187+
j := engine.NewJob(dbJob.Namespace, dbJob.Queue, dbJob.Body, uint32(dbJob.TTL(now)),
188+
uint32(dbJob.ReadyTime-now), uint16(dbJob.Tries), dbJob.JobID)
220189
jobs = append(jobs, j)
221190
return nil
222191
})

storage/persistence/spanner/spanner_test.go

Lines changed: 83 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -3,115 +3,108 @@ package spanner
33
import (
44
"context"
55
"fmt"
6-
"os"
76
"testing"
7+
"time"
88

99
"github.com/bitleak/lmstfy/config"
10+
"github.com/bitleak/lmstfy/engine"
11+
"github.com/bitleak/lmstfy/storage/persistence/model"
12+
1013
"github.com/stretchr/testify/assert"
14+
"github.com/stretchr/testify/require"
1115
)
1216

13-
var (
14-
dummyCtx = context.TODO()
17+
const (
18+
poolName = "test_pool"
19+
namespace = "test_ns"
1520
)
1621

17-
func init() {
18-
if os.Getenv("SPANNER_EMULATOR_HOST") == "" {
19-
panic(fmt.Sprintf("failed to find $SPANNER_EMULATOR_HOST value"))
20-
}
21-
err := CreateInstance(dummyCtx, config.SpannerEmulator)
22-
if err != nil {
23-
panic(fmt.Sprintf("create instance error: %v", err))
22+
func TestSpanner_Basic(t *testing.T) {
23+
ctx := context.Background()
24+
mgr, err := NewSpanner(config.SpannerEmulator)
25+
require.NoError(t, err)
26+
27+
jobCnt := int64(10)
28+
jobIDs := make([]string, jobCnt)
29+
createJobs := make([]engine.Job, jobCnt)
30+
for i := int64(0); i < jobCnt; i++ {
31+
queue := "q1"
32+
if i%2 == 0 {
33+
queue = "q2"
34+
}
35+
createJobs[i] = engine.NewJob(namespace, queue, []byte("hello"), 10, 4, 3, "")
36+
jobIDs[i] = createJobs[i].ID()
2437
}
25-
err = CreateDatabase(dummyCtx, config.SpannerEmulator)
26-
if err != nil {
27-
panic(fmt.Sprintf("create db error: %v", err))
38+
require.NoError(t, mgr.BatchAddJobs(ctx, poolName, createJobs))
39+
40+
validateJob := func(t *testing.T, job engine.Job) {
41+
assert.NotEmpty(t, job.ID())
42+
assert.EqualValues(t, job.Namespace(), namespace)
43+
assert.EqualValues(t, job.Tries(), 3)
44+
assert.GreaterOrEqual(t, job.Delay(), uint32(1))
45+
assert.LessOrEqual(t, job.Delay(), uint32(4))
46+
assert.GreaterOrEqual(t, job.TTL(), uint32(1))
47+
assert.LessOrEqual(t, job.TTL(), uint32(10))
2848
}
29-
}
3049

31-
func TestCreateSpannerClient(t *testing.T) {
32-
_, err := createSpannerClient(config.SpannerEmulator)
33-
assert.Nil(t, err)
34-
}
50+
t.Run("Batch Get Jobs By ID", func(t *testing.T) {
51+
jobs, err := mgr.BatchGetJobsByID(ctx, jobIDs)
52+
assert.Nil(t, err)
53+
assert.EqualValues(t, len(jobIDs), len(jobs))
54+
for _, job := range jobs {
55+
validateJob(t, job)
56+
}
57+
})
3558

36-
func TestSpanner_BatchAddDelJobs(t *testing.T) {
37-
mgr, err := NewSpanner(config.SpannerEmulator)
38-
if err != nil {
39-
panic(fmt.Sprintf("Failed to create spanner client with error: %s", err))
40-
}
41-
jobs := createTestJobsData()
42-
err = mgr.BatchAddJobs(ctx, poolName, jobs)
43-
if err != nil {
44-
panic(fmt.Sprintf("Failed to add jobs with error: %s", err))
45-
}
46-
t.Logf("add jobs success %v rows", len(jobs))
59+
t.Run("Get Ready Jobs", func(t *testing.T) {
60+
readyJobs, err := mgr.GetReadyJobs(ctx, &model.DBJobReq{
61+
PoolName: poolName,
62+
ReadyTime: time.Now().Unix() + 10,
63+
Count: jobCnt,
64+
})
65+
require.NoError(t, err)
66+
require.EqualValues(t, jobCnt, len(readyJobs))
67+
for _, job := range readyJobs {
68+
validateJob(t, job)
69+
}
70+
})
4771

48-
count, err := mgr.DelJobs(ctx, jobIDs)
49-
if err != nil {
50-
panic(fmt.Sprintf("failed to delete job: %v", err))
51-
}
52-
t.Logf("del jobs success %v rows", count)
53-
}
72+
t.Run("Get Queue Size", func(t *testing.T) {
73+
queueSizes, err := mgr.GetQueueSize(ctx, []*model.DBJobReq{
74+
{PoolName: poolName, Namespace: namespace, Queue: "q1", ReadyTime: time.Now().Unix() - 10, Count: jobCnt},
75+
{PoolName: poolName, Namespace: namespace, Queue: "q2", ReadyTime: time.Now().Unix() - 10, Count: jobCnt},
76+
})
77+
require.NoError(t, err)
78+
assert.EqualValues(t, jobCnt/2, queueSizes[fmt.Sprintf("%s/%s", namespace, "q1")])
79+
assert.EqualValues(t, jobCnt/2, queueSizes[fmt.Sprintf("%s/%s", namespace, "q2")])
80+
})
5481

55-
func TestSpanner_BatchGetJobs(t *testing.T) {
56-
mgr, err := NewSpanner(config.SpannerEmulator)
57-
if err != nil {
58-
panic(fmt.Sprintf("Failed to create spanner client with error: %s", err))
59-
}
60-
jobs := createTestJobsData()
61-
mgr.BatchAddJobs(ctx, poolName, jobs)
62-
req := createTestReqData()
63-
jobs, err = mgr.BatchGetJobs(ctx, req)
64-
if err != nil {
65-
panic(fmt.Sprintf("BatchGetJobs failed with error: %s", err))
66-
}
67-
assert.EqualValues(t, 3, len(jobs))
68-
mgr.DelJobs(ctx, jobIDs)
82+
t.Run("Del Jobs", func(t *testing.T) {
83+
count, err := mgr.DelJobs(context.Background(), jobIDs)
84+
require.NoError(t, err)
85+
require.EqualValues(t, jobCnt, count)
86+
})
6987
}
7088

71-
func TestSpanner_GetQueueSize(t *testing.T) {
89+
func TestSpanner_NoExpiredJob(t *testing.T) {
90+
ctx := context.Background()
7291
mgr, err := NewSpanner(config.SpannerEmulator)
73-
if err != nil {
74-
panic(fmt.Sprintf("Failed to create spanner client with error: %s", err))
75-
}
76-
jobs := createTestJobsData()
77-
mgr.BatchAddJobs(ctx, poolName, jobs)
78-
req := createTestReqData()
79-
count, err := mgr.GetQueueSize(ctx, req)
80-
if err != nil || len(count) == 0 {
81-
panic(fmt.Sprintf("BatchGetJobs failed with error: %s", err))
82-
}
83-
key1, key2 := fmt.Sprintf("%s/%s", "n1", "q1"), fmt.Sprintf("%s/%s", "n1", "q2")
84-
assert.EqualValues(t, 2, count[key1])
85-
assert.EqualValues(t, 1, count[key2])
86-
mgr.DelJobs(ctx, jobIDs)
87-
}
92+
require.NoError(t, err)
8893

89-
func TestSpanner_GetReadyJobs(t *testing.T) {
90-
mgr, err := NewSpanner(config.SpannerEmulator)
91-
if err != nil {
92-
panic(fmt.Sprintf("Failed to create spanner client with error: %s", err))
93-
}
94-
jobs := createTestJobsData()
95-
mgr.BatchAddJobs(ctx, poolName, jobs)
96-
req := createTestReqData2()
97-
jobs, err = mgr.GetReadyJobs(ctx, req)
98-
if err != nil {
99-
panic(fmt.Sprintf("GetReadyJobs failed with error: %s", err))
94+
jobCnt := int64(10)
95+
jobIDs := make([]string, jobCnt)
96+
createJobs := make([]engine.Job, jobCnt)
97+
for i := int64(0); i < jobCnt; i++ {
98+
queue := "q3"
99+
createJobs[i] = engine.NewJob(namespace, queue, []byte("hello"), 0, 4, 3, "")
100+
jobIDs[i] = createJobs[i].ID()
100101
}
101-
assert.EqualValues(t, 2, len(jobs))
102-
mgr.DelJobs(ctx, jobIDs)
103-
}
102+
require.NoError(t, mgr.BatchAddJobs(ctx, poolName, createJobs))
104103

105-
func TestSpanner_BatchGetJobsByID(t *testing.T) {
106-
mgr, err := NewSpanner(config.SpannerEmulator)
107-
if err != nil {
108-
panic(fmt.Sprintf("Failed to create spanner client with error: %s", err))
109-
}
110-
jobs := createTestJobsData()
111-
mgr.BatchAddJobs(ctx, poolName, jobs)
112-
IDs := []string{"1", "2", "3"}
113-
jobs, err = mgr.BatchGetJobsByID(ctx, IDs)
104+
jobs, err := mgr.BatchGetJobsByID(ctx, jobIDs)
114105
assert.Nil(t, err)
115-
assert.EqualValues(t, 3, len(jobs))
116-
mgr.DelJobs(ctx, jobIDs)
106+
assert.EqualValues(t, len(jobIDs), len(jobs))
107+
for _, job := range jobs {
108+
assert.EqualValues(t, job.TTL(), 0)
109+
}
117110
}

0 commit comments

Comments
 (0)