From 5942f326aeecb7b14e550b6d713752c8b7d3d87c Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Thu, 24 Feb 2022 15:30:09 -0500 Subject: [PATCH] api: paginated results with different ordering The paginator logic was built when go-memdb iterators would return items ordered lexicographically by their ID prefixes, but #12054 added the option for some tables to return results ordered by their `CreateIndex` instead, which invalidated the previous paginator assumption. The iterator used for pagination must still return results in some order so that the paginator can properly handle requests where the next_token value is not present in the results anymore (e.g., the eval was GC'ed). In these situations, the paginator will start the returned page in the first element right after where the requested token should've been. This commit moves the logic to generate pagination tokens from the elements being paginated to the iterator itself so that callers can have more control over the token format to make sure they are properly ordered and stable. It also allows configuring the paginator as being ordered in ascending or descending order, which is relevant when looking for a token that may not be present anymore. --- nomad/deployment_endpoint.go | 32 +++++++- nomad/deployment_endpoint_test.go | 59 +++++++++++--- nomad/eval_endpoint.go | 31 +++++++- nomad/eval_endpoint_test.go | 128 ++++++++++++++++++++---------- nomad/state/filter_test.go | 27 ++++++- nomad/state/paginator.go | 30 ++++--- nomad/state/paginator_test.go | 32 ++++---- nomad/state/schema.go | 36 +++++++-- nomad/structs/structs.go | 8 -- 9 files changed, 279 insertions(+), 104 deletions(-) diff --git a/nomad/deployment_endpoint.go b/nomad/deployment_endpoint.go index bfd3be7f4c0d..70f685d4a280 100644 --- a/nomad/deployment_endpoint.go +++ b/nomad/deployment_endpoint.go @@ -13,6 +13,30 @@ import ( "github.com/hashicorp/nomad/nomad/structs" ) +// DeploymentPaginationIterator is a wrapper over a go-memdb iterator that +// implements the paginator Iterator interface. +type DeploymentPaginationIterator struct { + iter memdb.ResultIterator + byCreateIndex bool +} + +func (it DeploymentPaginationIterator) Next() (string, interface{}) { + raw := it.iter.Next() + if raw == nil { + return "", nil + } + + d := raw.(*structs.Deployment) + token := d.ID + + // prefix the pagination token by CreateIndex to keep it properly sorted. + if it.byCreateIndex { + token = fmt.Sprintf("%v-%v", d.CreateIndex, d.ID) + } + + return token, d +} + // Deployment endpoint is used for manipulating deployments type Deployment struct { srv *Server @@ -409,20 +433,26 @@ func (d *Deployment) List(args *structs.DeploymentListRequest, reply *structs.De // Capture all the deployments var err error var iter memdb.ResultIterator + var deploymentIter DeploymentPaginationIterator if prefix := args.QueryOptions.Prefix; prefix != "" { iter, err = store.DeploymentsByIDPrefix(ws, namespace, prefix) + deploymentIter.byCreateIndex = false } else if namespace != structs.AllNamespacesSentinel { iter, err = store.DeploymentsByNamespaceOrdered(ws, namespace, args.Ascending) + deploymentIter.byCreateIndex = true } else { iter, err = store.Deployments(ws, args.Ascending) + deploymentIter.byCreateIndex = true } if err != nil { return err } + deploymentIter.iter = iter + var deploys []*structs.Deployment - paginator, err := state.NewPaginator(iter, args.QueryOptions, + paginator, err := state.NewPaginator(deploymentIter, args.QueryOptions, func(raw interface{}) error { deploy := raw.(*structs.Deployment) deploys = append(deploys, deploy) diff --git a/nomad/deployment_endpoint_test.go b/nomad/deployment_endpoint_test.go index 7170b507b3d6..e91bc28a8a77 100644 --- a/nomad/deployment_endpoint_test.go +++ b/nomad/deployment_endpoint_test.go @@ -1271,11 +1271,18 @@ func TestDeploymentEndpoint_List_Pagination(t *testing.T) { {id: "aaaaaabb-3350-4b4b-d185-0e1992ed43e9"}, // 4 {id: "aaaaaacc-3350-4b4b-d185-0e1992ed43e9"}, // 5 {id: "aaaaaadd-3350-4b4b-d185-0e1992ed43e9"}, // 6 + {id: "00000111-3350-4b4b-d185-0e1992ed43e9"}, // 7 + {}, // 8, index missing + {id: "bbbb1111-3350-4b4b-d185-0e1992ed43e9"}, // 9 } state := s1.fsm.State() for i, m := range mocks { + if m.id == "" { + continue + } + index := 1000 + uint64(i) deployment := mock.Deployment() deployment.Status = structs.DeploymentStatusCancelled @@ -1305,7 +1312,7 @@ func TestDeploymentEndpoint_List_Pagination(t *testing.T) { { name: "test01 size-2 page-1 default NS", pageSize: 2, - expectedNextToken: "aaaaaaaa-3350-4b4b-d185-0e1992ed43e9", + expectedNextToken: "1003-aaaaaaaa-3350-4b4b-d185-0e1992ed43e9", expectedIDs: []string{ "aaaa1111-3350-4b4b-d185-0e1992ed43e9", "aaaaaa22-3350-4b4b-d185-0e1992ed43e9", @@ -1315,7 +1322,7 @@ func TestDeploymentEndpoint_List_Pagination(t *testing.T) { name: "test02 size-2 page-1 default NS with prefix", prefix: "aaaa", pageSize: 2, - expectedNextToken: "aaaaaaaa-3350-4b4b-d185-0e1992ed43e9", + expectedNextToken: "aaaaaaaa-3350-4b4b-d185-0e1992ed43e9", // prefix results are not sorted by create index expectedIDs: []string{ "aaaa1111-3350-4b4b-d185-0e1992ed43e9", "aaaaaa22-3350-4b4b-d185-0e1992ed43e9", @@ -1324,8 +1331,8 @@ func TestDeploymentEndpoint_List_Pagination(t *testing.T) { { name: "test03 size-2 page-2 default NS", pageSize: 2, - nextToken: "aaaaaaaa-3350-4b4b-d185-0e1992ed43e9", - expectedNextToken: "aaaaaacc-3350-4b4b-d185-0e1992ed43e9", + nextToken: "1003-aaaaaaaa-3350-4b4b-d185-0e1992ed43e9", + expectedNextToken: "1005-aaaaaacc-3350-4b4b-d185-0e1992ed43e9", expectedIDs: []string{ "aaaaaaaa-3350-4b4b-d185-0e1992ed43e9", "aaaaaabb-3350-4b4b-d185-0e1992ed43e9", @@ -1343,14 +1350,25 @@ func TestDeploymentEndpoint_List_Pagination(t *testing.T) { }, }, { - name: "test05 no valid results with filters and prefix", + name: "test05 size-2 page-2 all namespaces", + namespace: "*", + pageSize: 2, + nextToken: "1002-aaaaaa33-3350-4b4b-d185-0e1992ed43e9", + expectedNextToken: "1004-aaaaaabb-3350-4b4b-d185-0e1992ed43e9", + expectedIDs: []string{ + "aaaaaa33-3350-4b4b-d185-0e1992ed43e9", + "aaaaaaaa-3350-4b4b-d185-0e1992ed43e9", + }, + }, + { + name: "test06 no valid results with filters and prefix", prefix: "cccc", pageSize: 2, nextToken: "", expectedIDs: []string{}, }, { - name: "test06 go-bexpr filter", + name: "test07 go-bexpr filter", namespace: "*", filter: `ID matches "^a+[123]"`, expectedIDs: []string{ @@ -1360,18 +1378,18 @@ func TestDeploymentEndpoint_List_Pagination(t *testing.T) { }, }, { - name: "test07 go-bexpr filter with pagination", + name: "test08 go-bexpr filter with pagination", namespace: "*", filter: `ID matches "^a+[123]"`, pageSize: 2, - expectedNextToken: "aaaaaa33-3350-4b4b-d185-0e1992ed43e9", + expectedNextToken: "1002-aaaaaa33-3350-4b4b-d185-0e1992ed43e9", expectedIDs: []string{ "aaaa1111-3350-4b4b-d185-0e1992ed43e9", "aaaaaa22-3350-4b4b-d185-0e1992ed43e9", }, }, { - name: "test08 go-bexpr filter in namespace", + name: "test09 go-bexpr filter in namespace", namespace: "non-default", filter: `Status == "cancelled"`, expectedIDs: []string{ @@ -1379,21 +1397,38 @@ func TestDeploymentEndpoint_List_Pagination(t *testing.T) { }, }, { - name: "test09 go-bexpr wrong namespace", + name: "test10 go-bexpr wrong namespace", namespace: "default", filter: `Namespace == "non-default"`, expectedIDs: []string{}, }, { - name: "test10 go-bexpr invalid expression", + name: "test11 go-bexpr invalid expression", filter: `NotValid`, expectedError: "failed to read filter expression", }, { - name: "test11 go-bexpr invalid field", + name: "test12 go-bexpr invalid field", filter: `InvalidField == "value"`, expectedError: "error finding value in datum", }, + { + name: "test13 non-lexicographic order", + pageSize: 1, + nextToken: "1007-00000111-3350-4b4b-d185-0e1992ed43e9", + expectedNextToken: "1009-bbbb1111-3350-4b4b-d185-0e1992ed43e9", + expectedIDs: []string{ + "00000111-3350-4b4b-d185-0e1992ed43e9", + }, + }, + { + name: "test14 missing index", + pageSize: 1, + nextToken: "1008-e9522802-0cd8-4b1d-9c9e-ab3d97938371", + expectedIDs: []string{ + "bbbb1111-3350-4b4b-d185-0e1992ed43e9", + }, + }, } for _, tc := range cases { diff --git a/nomad/eval_endpoint.go b/nomad/eval_endpoint.go index b938ec9280eb..0b6b26f598aa 100644 --- a/nomad/eval_endpoint.go +++ b/nomad/eval_endpoint.go @@ -21,6 +21,30 @@ const ( DefaultDequeueTimeout = time.Second ) +// EvalPaginationIterator is a wrapper over a go-memdb iterator that implements +// the paginator Iterator interface. +type EvalPaginationIterator struct { + iter memdb.ResultIterator + byCreateIndex bool +} + +func (it EvalPaginationIterator) Next() (string, interface{}) { + raw := it.iter.Next() + if raw == nil { + return "", nil + } + + eval := raw.(*structs.Evaluation) + token := eval.ID + + // prefix the pagination token by CreateIndex to keep it properly sorted. + if it.byCreateIndex { + token = fmt.Sprintf("%v-%v", eval.CreateIndex, eval.ID) + } + + return token, eval +} + // Eval endpoint is used for eval interactions type Eval struct { srv *Server @@ -414,13 +438,17 @@ func (e *Eval) List(args *structs.EvalListRequest, reply *structs.EvalListRespon // Scan all the evaluations var err error var iter memdb.ResultIterator + var evalIter EvalPaginationIterator if prefix := args.QueryOptions.Prefix; prefix != "" { iter, err = store.EvalsByIDPrefix(ws, namespace, prefix) + evalIter.byCreateIndex = false } else if namespace != structs.AllNamespacesSentinel { iter, err = store.EvalsByNamespaceOrdered(ws, namespace, args.Ascending) + evalIter.byCreateIndex = true } else { iter, err = store.Evals(ws, args.Ascending) + evalIter.byCreateIndex = true } if err != nil { return err @@ -432,9 +460,10 @@ func (e *Eval) List(args *structs.EvalListRequest, reply *structs.EvalListRespon } return false }) + evalIter.iter = iter var evals []*structs.Evaluation - paginator, err := state.NewPaginator(iter, args.QueryOptions, + paginator, err := state.NewPaginator(evalIter, args.QueryOptions, func(raw interface{}) error { eval := raw.(*structs.Evaluation) evals = append(evals, eval) diff --git a/nomad/eval_endpoint_test.go b/nomad/eval_endpoint_test.go index 3aa327508f16..92463394e7ee 100644 --- a/nomad/eval_endpoint_test.go +++ b/nomad/eval_endpoint_test.go @@ -1013,40 +1013,51 @@ func TestEvalEndpoint_List_PaginationFiltering(t *testing.T) { // in the order that the state store will return them from the // iterator (sorted by create index), for ease of writing tests mocks := []struct { - id string + ids []string namespace string jobID string status string }{ - {id: "aaaa1111-3350-4b4b-d185-0e1992ed43e9", jobID: "example"}, // 0 - {id: "aaaaaa22-3350-4b4b-d185-0e1992ed43e9", jobID: "example"}, // 1 - {id: "aaaaaa33-3350-4b4b-d185-0e1992ed43e9", namespace: "non-default"}, // 2 - {id: "aaaaaaaa-3350-4b4b-d185-0e1992ed43e9", jobID: "example", status: "blocked"}, // 3 - {id: "aaaaaabb-3350-4b4b-d185-0e1992ed43e9"}, // 4 - {id: "aaaaaacc-3350-4b4b-d185-0e1992ed43e9"}, // 5 - {id: "aaaaaadd-3350-4b4b-d185-0e1992ed43e9", jobID: "example"}, // 6 - {id: "aaaaaaee-3350-4b4b-d185-0e1992ed43e9", jobID: "example"}, // 7 - {id: "aaaaaaff-3350-4b4b-d185-0e1992ed43e9"}, // 8 + {ids: []string{"aaaa1111-3350-4b4b-d185-0e1992ed43e9"}, jobID: "example"}, // 0 + {ids: []string{"aaaaaa22-3350-4b4b-d185-0e1992ed43e9"}, jobID: "example"}, // 1 + {ids: []string{"aaaaaa33-3350-4b4b-d185-0e1992ed43e9"}, namespace: "non-default"}, // 2 + {ids: []string{"aaaaaaaa-3350-4b4b-d185-0e1992ed43e9"}, jobID: "example", status: "blocked"}, // 3 + {ids: []string{"aaaaaabb-3350-4b4b-d185-0e1992ed43e9"}}, // 4 + {ids: []string{"aaaaaacc-3350-4b4b-d185-0e1992ed43e9"}}, // 5 + {ids: []string{"aaaaaadd-3350-4b4b-d185-0e1992ed43e9"}, jobID: "example"}, // 6 + {ids: []string{"aaaaaaee-3350-4b4b-d185-0e1992ed43e9"}, jobID: "example"}, // 7 + {ids: []string{"aaaaaaff-3350-4b4b-d185-0e1992ed43e9"}}, // 8 + {ids: []string{"00000111-3350-4b4b-d185-0e1992ed43e9"}}, // 9 + {ids: []string{ // 10 + "00000222-3350-4b4b-d185-0e1992ed43e9", + "00000333-3350-4b4b-d185-0e1992ed43e9", + }}, + {}, // 11, index missing + {ids: []string{"bbbb1111-3350-4b4b-d185-0e1992ed43e9"}}, // 12 } state := s1.fsm.State() var evals []*structs.Evaluation for i, m := range mocks { - eval := mock.Eval() - eval.ID = m.id - if m.namespace != "" { // defaults to "default" - eval.Namespace = m.namespace - } - if m.jobID != "" { // defaults to some random UUID - eval.JobID = m.jobID - } - if m.status != "" { // defaults to "pending" - eval.Status = m.status + evalsInTx := []*structs.Evaluation{} + for _, id := range m.ids { + eval := mock.Eval() + eval.ID = id + if m.namespace != "" { // defaults to "default" + eval.Namespace = m.namespace + } + if m.jobID != "" { // defaults to some random UUID + eval.JobID = m.jobID + } + if m.status != "" { // defaults to "pending" + eval.Status = m.status + } + evals = append(evals, eval) + evalsInTx = append(evalsInTx, eval) } - evals = append(evals, eval) index := 1000 + uint64(i) - require.NoError(t, state.UpsertEvals(structs.MsgTypeTestSetup, index, []*structs.Evaluation{eval})) + require.NoError(t, state.UpsertEvals(structs.MsgTypeTestSetup, index, evalsInTx)) } aclToken := mock.CreatePolicyAndToken(t, state, 1100, "test-valid-read", @@ -1073,13 +1084,13 @@ func TestEvalEndpoint_List_PaginationFiltering(t *testing.T) { "aaaa1111-3350-4b4b-d185-0e1992ed43e9", "aaaaaa22-3350-4b4b-d185-0e1992ed43e9", }, - expectedNextToken: "aaaaaaaa-3350-4b4b-d185-0e1992ed43e9", // next one in default namespace + expectedNextToken: "1003-aaaaaaaa-3350-4b4b-d185-0e1992ed43e9", // next one in default namespace }, { name: "test02 size-2 page-1 default NS with prefix", prefix: "aaaa", pageSize: 2, - expectedNextToken: "aaaaaaaa-3350-4b4b-d185-0e1992ed43e9", + expectedNextToken: "aaaaaaaa-3350-4b4b-d185-0e1992ed43e9", // prefix results are not sorted by create index expectedIDs: []string{ "aaaa1111-3350-4b4b-d185-0e1992ed43e9", "aaaaaa22-3350-4b4b-d185-0e1992ed43e9", @@ -1088,8 +1099,8 @@ func TestEvalEndpoint_List_PaginationFiltering(t *testing.T) { { name: "test03 size-2 page-2 default NS", pageSize: 2, - nextToken: "aaaaaaaa-3350-4b4b-d185-0e1992ed43e9", - expectedNextToken: "aaaaaacc-3350-4b4b-d185-0e1992ed43e9", + nextToken: "1003-aaaaaaaa-3350-4b4b-d185-0e1992ed43e9", + expectedNextToken: "1005-aaaaaacc-3350-4b4b-d185-0e1992ed43e9", expectedIDs: []string{ "aaaaaaaa-3350-4b4b-d185-0e1992ed43e9", "aaaaaabb-3350-4b4b-d185-0e1992ed43e9", @@ -1112,7 +1123,7 @@ func TestEvalEndpoint_List_PaginationFiltering(t *testing.T) { filterJobID: "example", filterStatus: "pending", // aaaaaaaa, bb, and cc are filtered by status - expectedNextToken: "aaaaaadd-3350-4b4b-d185-0e1992ed43e9", + expectedNextToken: "1006-aaaaaadd-3350-4b4b-d185-0e1992ed43e9", expectedIDs: []string{ "aaaa1111-3350-4b4b-d185-0e1992ed43e9", "aaaaaa22-3350-4b4b-d185-0e1992ed43e9", @@ -1148,7 +1159,7 @@ func TestEvalEndpoint_List_PaginationFiltering(t *testing.T) { pageSize: 3, // reads off the end filterJobID: "example", filterStatus: "pending", - nextToken: "aaaaaaaa-3350-4b4b-d185-0e1992ed43e9", + nextToken: "1003-aaaaaaaa-3350-4b4b-d185-0e1992ed43e9", expectedNextToken: "", expectedIDs: []string{ "aaaaaadd-3350-4b4b-d185-0e1992ed43e9", @@ -1169,14 +1180,25 @@ func TestEvalEndpoint_List_PaginationFiltering(t *testing.T) { }, }, { - name: "test10 no valid results with filters", + name: "test10 size-2 page-2 all namespaces", + namespace: "*", + pageSize: 2, + nextToken: "1002-aaaaaa33-3350-4b4b-d185-0e1992ed43e9", + expectedNextToken: "1004-aaaaaabb-3350-4b4b-d185-0e1992ed43e9", + expectedIDs: []string{ + "aaaaaa33-3350-4b4b-d185-0e1992ed43e9", + "aaaaaaaa-3350-4b4b-d185-0e1992ed43e9", + }, + }, + { + name: "test11 no valid results with filters", pageSize: 2, filterJobID: "whatever", nextToken: "", expectedIDs: []string{}, }, { - name: "test11 no valid results with filters and prefix", + name: "test12 no valid results with filters and prefix", prefix: "aaaa", pageSize: 2, filterJobID: "whatever", @@ -1184,36 +1206,36 @@ func TestEvalEndpoint_List_PaginationFiltering(t *testing.T) { expectedIDs: []string{}, }, { - name: "test12 no valid results with filters page-2", + name: "test13 no valid results with filters page-2", filterJobID: "whatever", nextToken: "aaaaaa11-3350-4b4b-d185-0e1992ed43e9", expectedIDs: []string{}, }, { - name: "test13 no valid results with filters page-2 with prefix", + name: "test14 no valid results with filters page-2 with prefix", prefix: "aaaa", filterJobID: "whatever", nextToken: "aaaaaa11-3350-4b4b-d185-0e1992ed43e9", expectedIDs: []string{}, }, { - name: "test14 go-bexpr filter", + name: "test15 go-bexpr filter", filter: `Status == "blocked"`, nextToken: "", expectedIDs: []string{"aaaaaaaa-3350-4b4b-d185-0e1992ed43e9"}, }, { - name: "test15 go-bexpr filter with pagination", + name: "test16 go-bexpr filter with pagination", filter: `JobID == "example"`, pageSize: 2, - expectedNextToken: "aaaaaaaa-3350-4b4b-d185-0e1992ed43e9", + expectedNextToken: "1003-aaaaaaaa-3350-4b4b-d185-0e1992ed43e9", expectedIDs: []string{ "aaaa1111-3350-4b4b-d185-0e1992ed43e9", "aaaaaa22-3350-4b4b-d185-0e1992ed43e9", }, }, { - name: "test16 go-bexpr filter namespace", + name: "test17 go-bexpr filter namespace", namespace: "non-default", filter: `ID contains "aaa"`, expectedIDs: []string{ @@ -1221,27 +1243,53 @@ func TestEvalEndpoint_List_PaginationFiltering(t *testing.T) { }, }, { - name: "test17 go-bexpr wrong namespace", + name: "test18 go-bexpr wrong namespace", namespace: "default", filter: `Namespace == "non-default"`, expectedIDs: []string{}, }, { - name: "test18 incompatible filtering", + name: "test19 incompatible filtering", filter: `JobID == "example"`, filterStatus: "complete", expectedError: structs.ErrIncompatibleFiltering.Error(), }, { - name: "test19 go-bexpr invalid expression", + name: "test20 go-bexpr invalid expression", filter: `NotValid`, expectedError: "failed to read filter expression", }, { - name: "test20 go-bexpr invalid field", + name: "test21 go-bexpr invalid field", filter: `InvalidField == "value"`, expectedError: "error finding value in datum", }, + { + name: "test22 non-lexicographic order", + pageSize: 1, + nextToken: "1009-00000111-3350-4b4b-d185-0e1992ed43e9", + expectedNextToken: "1010-00000222-3350-4b4b-d185-0e1992ed43e9", + expectedIDs: []string{ + "00000111-3350-4b4b-d185-0e1992ed43e9", + }, + }, + { + name: "test23 same index", + pageSize: 1, + nextToken: "1010-00000222-3350-4b4b-d185-0e1992ed43e9", + expectedNextToken: "1010-00000333-3350-4b4b-d185-0e1992ed43e9", + expectedIDs: []string{ + "00000222-3350-4b4b-d185-0e1992ed43e9", + }, + }, + { + name: "test24 missing index", + pageSize: 1, + nextToken: "1011-e9522802-0cd8-4b1d-9c9e-ab3d97938371", + expectedIDs: []string{ + "bbbb1111-3350-4b4b-d185-0e1992ed43e9", + }, + }, } for _, tc := range cases { diff --git a/nomad/state/filter_test.go b/nomad/state/filter_test.go index f0ba14a73b6b..2fa1b02ad3e9 100644 --- a/nomad/state/filter_test.go +++ b/nomad/state/filter_test.go @@ -5,6 +5,7 @@ import ( "time" "github.com/hashicorp/go-bexpr" + memdb "github.com/hashicorp/go-memdb" "github.com/hashicorp/nomad/helper/uuid" "github.com/hashicorp/nomad/nomad/structs" ) @@ -75,8 +76,9 @@ func BenchmarkEvalListFilter(b *testing.B) { for i := 0; i < b.N; i++ { iter, _ := state.EvalsByNamespace(nil, structs.DefaultNamespace) + evalIter := evalPaginationIterator{iter} var evals []*structs.Evaluation - paginator, err := NewPaginator(iter, opts, func(raw interface{}) error { + paginator, err := NewPaginator(evalIter, opts, func(raw interface{}) error { eval := raw.(*structs.Evaluation) evals = append(evals, eval) return nil @@ -98,8 +100,9 @@ func BenchmarkEvalListFilter(b *testing.B) { for i := 0; i < b.N; i++ { iter, _ := state.Evals(nil, false) + evalIter := evalPaginationIterator{iter} var evals []*structs.Evaluation - paginator, err := NewPaginator(iter, opts, func(raw interface{}) error { + paginator, err := NewPaginator(evalIter, opts, func(raw interface{}) error { eval := raw.(*structs.Evaluation) evals = append(evals, eval) return nil @@ -134,8 +137,9 @@ func BenchmarkEvalListFilter(b *testing.B) { for i := 0; i < b.N; i++ { iter, _ := state.EvalsByNamespace(nil, structs.DefaultNamespace) + evalIter := evalPaginationIterator{iter} var evals []*structs.Evaluation - paginator, err := NewPaginator(iter, opts, func(raw interface{}) error { + paginator, err := NewPaginator(evalIter, opts, func(raw interface{}) error { eval := raw.(*structs.Evaluation) evals = append(evals, eval) return nil @@ -171,8 +175,9 @@ func BenchmarkEvalListFilter(b *testing.B) { for i := 0; i < b.N; i++ { iter, _ := state.Evals(nil, false) + evalIter := evalPaginationIterator{iter} var evals []*structs.Evaluation - paginator, err := NewPaginator(iter, opts, func(raw interface{}) error { + paginator, err := NewPaginator(evalIter, opts, func(raw interface{}) error { eval := raw.(*structs.Evaluation) evals = append(evals, eval) return nil @@ -230,3 +235,17 @@ func generateEval(i int, ns string) *structs.Evaluation { ModifyTime: now, } } + +type evalPaginationIterator struct { + iter memdb.ResultIterator +} + +func (it evalPaginationIterator) Next() (string, interface{}) { + raw := it.iter.Next() + if raw == nil { + return "", nil + } + + eval := raw.(*structs.Evaluation) + return eval.ID, eval +} diff --git a/nomad/state/paginator.go b/nomad/state/paginator.go index 02f7f6fa8c55..607ff8cde07a 100644 --- a/nomad/state/paginator.go +++ b/nomad/state/paginator.go @@ -9,9 +9,12 @@ import ( // Iterator is the interface that must be implemented to use the Paginator. type Iterator interface { - // Next returns the next element to be considered for pagination. + // Next returns the next element to be considered for pagination along with + // a token string used to uniquely identify elements in the iteration. // The page will end if nil is returned. - Next() interface{} + // Tokens should have a stable order and the order must match the paginator + // ascending property. + Next() (string, interface{}) } // Paginator is an iterator over a memdb.ResultIterator that returns @@ -22,6 +25,7 @@ type Paginator struct { itemCount int32 seekingToken string nextToken string + ascending bool nextTokenFound bool pageErr error @@ -50,6 +54,7 @@ func NewPaginator(iter Iterator, opts structs.QueryOptions, appendFunc func(inte iter: iter, perPage: opts.PerPage, seekingToken: opts.NextToken, + ascending: opts.Ascending, nextTokenFound: opts.NextToken == "", filterEvaluator: evaluator, appendFunc: appendFunc, @@ -79,16 +84,23 @@ DONE: } func (p *Paginator) next() (interface{}, paginatorState) { - raw := p.iter.Next() + token, raw := p.iter.Next() if raw == nil { p.nextToken = "" return nil, paginatorComplete } // have we found the token we're seeking (if any)? - id := raw.(IDGetter).GetID() - p.nextToken = id - if !p.nextTokenFound && id < p.seekingToken { + p.nextToken = token + + var passedToken bool + if p.ascending { + passedToken = token < p.seekingToken + } else { + passedToken = token > p.seekingToken + } + + if !p.nextTokenFound && passedToken { return nil, paginatorSkip } @@ -115,12 +127,6 @@ func (p *Paginator) next() (interface{}, paginatorState) { return raw, paginatorInclude } -// IDGetter must be implemented for the results of any iterator we -// want to paginate -type IDGetter interface { - GetID() string -} - type paginatorState int const ( diff --git a/nomad/state/paginator_test.go b/nomad/state/paginator_test.go index b0871ddd3fbc..0d6f07fdac97 100644 --- a/nomad/state/paginator_test.go +++ b/nomad/state/paginator_test.go @@ -6,7 +6,6 @@ import ( "github.com/stretchr/testify/require" - memdb "github.com/hashicorp/go-memdb" "github.com/hashicorp/nomad/nomad/structs" ) @@ -63,7 +62,9 @@ func TestPaginator(t *testing.T) { paginator, err := NewPaginator(iter, structs.QueryOptions{ - PerPage: tc.perPage, NextToken: tc.nextToken, + PerPage: tc.perPage, + NextToken: tc.nextToken, + Ascending: true, }, func(raw interface{}) error { if tc.expectedError != "" { @@ -71,7 +72,7 @@ func TestPaginator(t *testing.T) { } result := raw.(*mockObject) - results = append(results, result.GetID()) + results = append(results, result.id) return nil }, ) @@ -96,32 +97,27 @@ func TestPaginator(t *testing.T) { // implements memdb.ResultIterator interface type testResultIterator struct { results chan interface{} - idx int } -func (i testResultIterator) Next() interface{} { +func (i testResultIterator) Next() (string, interface{}) { select { - case result := <-i.results: - return result + case raw := <-i.results: + if raw == nil { + return "", nil + } + + m := raw.(*mockObject) + return m.id, m default: - return nil + return "", nil } } -// not used, but required to implement memdb.ResultIterator -func (i testResultIterator) WatchCh() <-chan struct{} { - return make(<-chan struct{}) -} - type mockObject struct { id string } -func (m *mockObject) GetID() string { - return m.id -} - -func newTestIterator(ids []string) memdb.ResultIterator { +func newTestIterator(ids []string) testResultIterator { iter := testResultIterator{results: make(chan interface{}, 20)} for _, id := range ids { iter.results <- &mockObject{id: id} diff --git a/nomad/state/schema.go b/nomad/state/schema.go index eb6805f04ab7..5c62ae2fde02 100644 --- a/nomad/state/schema.go +++ b/nomad/state/schema.go @@ -320,9 +320,16 @@ func deploymentSchema() *memdb.TableSchema { "create": { Name: "create", AllowMissing: false, - Unique: false, - Indexer: &memdb.UintFieldIndex{ - Field: "CreateIndex", + Unique: true, + Indexer: &memdb.CompoundIndex{ + Indexes: []memdb.Indexer{ + &memdb.UintFieldIndex{ + Field: "CreateIndex", + }, + &memdb.StringFieldIndex{ + Field: "ID", + }, + }, }, }, @@ -346,7 +353,7 @@ func deploymentSchema() *memdb.TableSchema { "namespace_create": { Name: "namespace_create", AllowMissing: false, - Unique: false, + Unique: true, Indexer: &memdb.CompoundIndex{ AllowMissing: false, Indexes: []memdb.Indexer{ @@ -356,6 +363,9 @@ func deploymentSchema() *memdb.TableSchema { &memdb.UintFieldIndex{ Field: "CreateIndex", }, + &memdb.StringFieldIndex{ + Field: "ID", + }, }, }, }, @@ -438,9 +448,16 @@ func evalTableSchema() *memdb.TableSchema { "create": { Name: "create", AllowMissing: false, - Unique: false, - Indexer: &memdb.UintFieldIndex{ - Field: "CreateIndex", + Unique: true, + Indexer: &memdb.CompoundIndex{ + Indexes: []memdb.Indexer{ + &memdb.UintFieldIndex{ + Field: "CreateIndex", + }, + &memdb.StringFieldIndex{ + Field: "ID", + }, + }, }, }, @@ -486,7 +503,7 @@ func evalTableSchema() *memdb.TableSchema { "namespace_create": { Name: "namespace_create", AllowMissing: false, - Unique: false, + Unique: true, Indexer: &memdb.CompoundIndex{ AllowMissing: false, Indexes: []memdb.Indexer{ @@ -496,6 +513,9 @@ func evalTableSchema() *memdb.TableSchema { &memdb.UintFieldIndex{ Field: "CreateIndex", }, + &memdb.StringFieldIndex{ + Field: "ID", + }, }, }, }, diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 9a30381cf537..739340105df6 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -10548,14 +10548,6 @@ type Evaluation struct { ModifyTime int64 } -// GetID implements the IDGetter interface, required for pagination -func (e *Evaluation) GetID() string { - if e == nil { - return "" - } - return e.ID -} - // TerminalStatus returns if the current status is terminal and // will no longer transition. func (e *Evaluation) TerminalStatus() bool {