Skip to content

Commit

Permalink
api: paginated results with different ordering
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@lgfa29
lgfa29 committed Feb 28, 2022
1 parent 636345a commit 5942f32
Show file tree
Hide file tree
Showing 9 changed files with 279 additions and 104 deletions.
32 changes: 31 additions & 1 deletion nomad/deployment_endpoint.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
59 changes: 47 additions & 12 deletions nomad/deployment_endpoint_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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{
Expand All @@ -1360,40 +1378,57 @@ 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{
"aaaaaa33-3350-4b4b-d185-0e1992ed43e9",
},
},
{
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 {
Expand Down
31 changes: 30 additions & 1 deletion nomad/eval_endpoint.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down

0 comments on commit 5942f32

Please sign in to comment.