From 839f5b3a092b4282722fae845cc9254ab3350f20 Mon Sep 17 00:00:00 2001 From: Alex Freidah Date: Wed, 24 Jun 2026 04:09:11 -0700 Subject: [PATCH] core: paginate /v1/jobs/statuses with a ModifyIndex+Namespace+ID cursor ModifyIndex is not unique across jobs, so once the jobs modify_index index became non-unique (#28158) the /v1/jobs/statuses pagination cursor (ModifyIndexTokenizer) could no longer identify a unique position: jobs sharing a ModifyIndex were returned on more than one page, and a group larger than per_page pinned the cursor so older jobs were unreachable. Add ModifyIndexAndNamespaceIDTokenizer, which tokenizes on ModifyIndex + Namespace + ID -- matching the memdb iteration order of the non-unique index, which breaks ties on the (Namespace, ID) primary key -- with a legacy bare-integer fallback for rolling upgrades, and use it for Job.Statuses. Retire the now-unused ModifyIndexTokenizer. On the web UI, treat the page token as opaque: navigate with a history stack instead of doing arithmetic on the cursor, and only synthesize a cursor for the "last" page. Fixes #28167 --- .changelog/28178.txt | 3 + nomad/job_endpoint_statuses.go | 2 +- .../job_endpoint_statuses_pagination_test.go | 86 ++++++++++++ nomad/job_endpoint_statuses_test.go | 21 +-- nomad/state/paginator/paginator_test.go | 12 +- nomad/state/paginator/tokenizer.go | 57 ++++++-- nomad/state/paginator/tokenizer_test.go | 129 +++++++++++++++++- ui/app/controllers/jobs/index.js | 81 +++++------ ui/mirage/config.js | 43 +++++- ui/tests/acceptance/jobs-list-test.js | 47 +++++++ 10 files changed, 410 insertions(+), 71 deletions(-) create mode 100644 .changelog/28178.txt create mode 100644 nomad/job_endpoint_statuses_pagination_test.go diff --git a/.changelog/28178.txt b/.changelog/28178.txt new file mode 100644 index 00000000000..3aad3e23d47 --- /dev/null +++ b/.changelog/28178.txt @@ -0,0 +1,3 @@ +```release-note:bug +core: Fixed a bug where jobs sharing a ModifyIndex could break /v1/jobs/statuses pagination, causing the jobs index page to show a job on more than one page, fail to page past a group of such jobs, or jump to an incorrect last page +``` diff --git a/nomad/job_endpoint_statuses.go b/nomad/job_endpoint_statuses.go index 7b8c7079dbc..e23b600b5cd 100644 --- a/nomad/job_endpoint_statuses.go +++ b/nomad/job_endpoint_statuses.go @@ -139,7 +139,7 @@ func (j *Job) Statuses( newJobs := set.New[structs.NamespacedID](0) pager, err := paginator.NewPaginator(iter, args.QueryOptions, selector, - paginator.ModifyIndexTokenizer[*structs.Job](args.NextToken), + paginator.ModifyIndexAndNamespaceIDTokenizer[*structs.Job](args.NextToken), func(job *structs.Job) (structs.JobStatusesJob, error) { var none structs.JobStatusesJob // this is where the sausage is made diff --git a/nomad/job_endpoint_statuses_pagination_test.go b/nomad/job_endpoint_statuses_pagination_test.go new file mode 100644 index 00000000000..b784c0fdbd2 --- /dev/null +++ b/nomad/job_endpoint_statuses_pagination_test.go @@ -0,0 +1,86 @@ +// Copyright IBM Corp. 2015, 2026 +// SPDX-License-Identifier: BUSL-1.1 + +package nomad + +import ( + "fmt" + "testing" + + "github.com/hashicorp/nomad/ci" + "github.com/hashicorp/nomad/nomad/mock" + "github.com/hashicorp/nomad/nomad/structs" + "github.com/hashicorp/nomad/testutil" + "github.com/shoenig/test/must" +) + +// TestJob_Statuses_Pagination_SharedModifyIndex is a regression test for the +// /v1/jobs/statuses paginator. Its NextToken is built from ModifyIndex alone, +// with no tiebreaker. When several jobs share a ModifyIndex (e.g. written in a +// single Raft transaction) and that group does not fit in one page, the cursor +// cannot advance past the group: the same page repeats (duplicate jobs) and the +// walk never terminates. +// +// Paginating this endpoint must always (a) terminate and (b) return every job +// exactly once, regardless of jobs sharing a ModifyIndex. +func TestJob_Statuses_Pagination_SharedModifyIndex(t *testing.T) { + ci.Parallel(t) + + s, cleanup := TestServer(t, func(c *Config) { + c.NumSchedulers = 0 + }) + t.Cleanup(cleanup) + testutil.WaitForLeader(t, s.RPC) + + // Write several jobs at a single shared ModifyIndex, as happens when one + // Raft transaction writes multiple jobs at once. + const n = 5 + const sharedIndex = 1000 + want := make(map[string]bool, n) + for i := range n { + job := mock.MinJob() + job.ID = fmt.Sprintf("shared-%02d", i) + must.NoError(t, s.State().UpsertJob(structs.MsgTypeTestSetup, sharedIndex, nil, job)) + want[job.ID] = true + } + + // Page through the endpoint following NextToken, the way the web UI does. + // per_page is smaller than the shared group, so the group spans a page + // boundary -- exactly the case the ModifyIndex-only token cannot handle. + seen := make(map[string]int, n) + token := "" + pages := 0 + // A terminating walk needs ceil(n/perPage) pages; a stalled cursor never + // stops, so anything well past that means the walk did not terminate. + maxPages := n * 3 + for { + pages++ + if pages > maxPages { + t.Fatalf("pagination did not terminate after %d pages: cursor stuck at token %q (seen=%v)", + pages, token, seen) + } + + req := &structs.JobStatusesRequest{} + req.QueryOptions.Region = "global" + req.QueryOptions.Namespace = structs.DefaultNamespace + req.QueryOptions.PerPage = 2 + req.QueryOptions.NextToken = token + + var resp structs.JobStatusesResponse + must.NoError(t, s.RPC("Job.Statuses", req, &resp)) + + for _, j := range resp.Jobs { + seen[j.ID]++ + } + if resp.NextToken == "" { + break + } + token = resp.NextToken + } + + for id := range want { + must.Eq(t, 1, seen[id], + must.Sprintf("job %s returned %d time(s) across pages, want exactly 1", id, seen[id])) + } + must.Eq(t, n, len(seen), must.Sprintf("walked jobs = %v", seen)) +} diff --git a/nomad/job_endpoint_statuses_test.go b/nomad/job_endpoint_statuses_test.go index 8844b6c59d6..219449a6da3 100644 --- a/nomad/job_endpoint_statuses_test.go +++ b/nomad/job_endpoint_statuses_test.go @@ -5,7 +5,7 @@ package nomad import ( "context" - "strconv" + "fmt" "testing" "time" @@ -142,12 +142,17 @@ func TestJob_Statuses(t *testing.T) { // test various single-job requests + // the pagination cursor is ".." + tok := func(j *structs.Job) string { + return fmt.Sprintf("%d.%s.%s", j.ModifyIndex, j.Namespace, j.ID) + } + for _, tc := range []struct { name string qo structs.QueryOptions jobs []structs.NamespacedID expect *structs.Job - expectNext uint64 // NextToken (ModifyIndex) + expectNext *structs.Job // job the NextToken cursor points at }{ { name: "page 1", @@ -155,16 +160,16 @@ func TestJob_Statuses(t *testing.T) { PerPage: 1, }, expect: jobs[4], - expectNext: jobs[3].ModifyIndex, + expectNext: jobs[3], }, { name: "page 2", qo: structs.QueryOptions{ PerPage: 1, - NextToken: strconv.FormatUint(jobs[3].ModifyIndex, 10), + NextToken: tok(jobs[3]), }, expect: jobs[3], - expectNext: jobs[2].ModifyIndex, + expectNext: jobs[2], }, { name: "reverse", @@ -173,7 +178,7 @@ func TestJob_Statuses(t *testing.T) { Reverse: true, }, expect: jobs[0], - expectNext: jobs[1].ModifyIndex, + expectNext: jobs[1], }, { name: "filter", @@ -212,8 +217,8 @@ func TestJob_Statuses(t *testing.T) { must.Eq(t, tc.expect.ID, resp.Jobs[0].ID) } expectToken := "" - if tc.expectNext > 0 { - expectToken = strconv.FormatUint(tc.expectNext, 10) + if tc.expectNext != nil { + expectToken = tok(tc.expectNext) } must.Eq(t, expectToken, resp.NextToken) }) diff --git a/nomad/state/paginator/paginator_test.go b/nomad/state/paginator/paginator_test.go index d3fd163ff1b..35aaae934dc 100644 --- a/nomad/state/paginator/paginator_test.go +++ b/nomad/state/paginator/paginator_test.go @@ -60,20 +60,22 @@ func TestPaginator(t *testing.T) { { name: "when numbers are numbers", perPage: 2, - // "10" is converted to uint64(10) and compared with uint64 index + // a bare "10" target exercises the legacy index-only path: "10" is + // parsed as uint64(10) and compared numerically with the index. nextToken: "10", - tokenizer: ModifyIndexTokenizer[*mockObject]("10"), + tokenizer: ModifyIndexAndNamespaceIDTokenizer[*mockObject]("10"), expected: []string{"10", "11"}, expectedNextToken: "", }, { name: "when zero is a number", perPage: 2, - // "" is converted to uint64(0) and compared with uint64 index + // an empty token starts from the beginning; the next token is the + // full ".." cursor (namespace is empty here). nextToken: "", - tokenizer: ModifyIndexTokenizer[*mockObject](""), + tokenizer: ModifyIndexAndNamespaceIDTokenizer[*mockObject](""), expected: []string{"0", "1"}, - expectedNextToken: "2", + expectedNextToken: "2..2", }, { name: "starting off the end", diff --git a/nomad/state/paginator/tokenizer.go b/nomad/state/paginator/tokenizer.go index 6823b223f93..4aa3440aeb4 100644 --- a/nomad/state/paginator/tokenizer.go +++ b/nomad/state/paginator/tokenizer.go @@ -75,17 +75,49 @@ func CreateIndexAndIDTokenizer[T idAndCreateIndexGetter](target string) Tokenize } } -// ModifyIndexTokenizer returns a tokenizer by ModifyIndex. -func ModifyIndexTokenizer[T modifyIndexGetter](target string) Tokenizer[T] { - // attempt to convert token to uint for iterators ordered numerically. - // it's safe to ignore the error here because the `next` method ignores - // this field for string tokens and 0 is valid for an unset numeric token. - targetIndex, _ := strconv.ParseUint(target, 10, 64) - +// ModifyIndexAndNamespaceIDTokenizer returns a tokenizer by ModifyIndex, with +// Namespace and ID as a tiebreaker. ModifyIndex is not unique across objects +// (several may be written in one Raft transaction), so ModifyIndex alone does +// not identify a unique position to resume pagination from. Namespace and ID +// make the token a total order that matches the memdb iteration order of the +// non-unique modify_index index, which breaks ties on the (Namespace, ID) +// primary key. +func ModifyIndexAndNamespaceIDTokenizer[T modifyIndexAndNamespaceIDGetter](target string) Tokenizer[T] { return func(item T) (string, int) { index := item.GetModifyIndex() - token := fmt.Sprintf("%d", index) - return token, cmp.Compare(index, targetIndex) + ns := item.GetNamespace() + id := item.GetID() + token := fmt.Sprintf("%d.%s.%s", index, ns, id) + + // Namespace cannot contain '.', so the index and namespace are the first + // two segments; the ID (which may contain '.') is the remainder. + parts := strings.SplitN(target, ".", 3) + + // Parse the index numerically so values like 12 and 102 compare + // correctly rather than lexicographically. A target that doesn't begin + // with an integer is malformed; fall back to a direct string compare. + targetIndex, err := strconv.ParseUint(parts[0], 10, 64) + if err != nil { + return token, cmp.Compare(token, target) + } + if c := cmp.Compare(index, targetIndex); c != 0 { + return token, c + } + + // Indexes are equal; break the tie by namespace then ID. A target with + // fewer segments (e.g. a legacy bare-integer token from before this + // tiebreaker existed, seen during a rolling upgrade) compares only the + // segments it has, degrading to the previous index-only behavior. + if len(parts) < 2 { + return token, 0 + } + if c := cmp.Compare(ns, parts[1]); c != 0 { + return token, c + } + if len(parts) < 3 { + return token, 0 + } + return token, cmp.Compare(id, parts[2]) } } @@ -120,3 +152,10 @@ type idAndCreateIndexGetter interface { type modifyIndexGetter interface { GetModifyIndex() uint64 } + +// modifyIndexAndNamespaceIDGetter must be implemented by structs that want to +// use ModifyIndex with a Namespace and ID tiebreaker as their pagination token. +type modifyIndexAndNamespaceIDGetter interface { + modifyIndexGetter + namespaceIDGetter +} diff --git a/nomad/state/paginator/tokenizer_test.go b/nomad/state/paginator/tokenizer_test.go index 244ee5ce009..9fbb217f558 100644 --- a/nomad/state/paginator/tokenizer_test.go +++ b/nomad/state/paginator/tokenizer_test.go @@ -39,9 +39,9 @@ func TestTokenizer(t *testing.T) { expected: fmt.Sprintf("%v.%v", j.CreateIndex, j.ID), }, { - name: "ModifyIndex", - tokenizer: ModifyIndexTokenizer[*structs.Job](""), - expected: fmt.Sprintf("%d", j.ModifyIndex), + name: "ModifyIndex.Namespace.ID", + tokenizer: ModifyIndexAndNamespaceIDTokenizer[*structs.Job](""), + expected: fmt.Sprintf("%d.%v.%v", j.ModifyIndex, j.Namespace, j.ID), }, } @@ -143,3 +143,126 @@ func (m *mockCreateIndexObject) GetCreateIndex() uint64 { func (m *mockCreateIndexObject) GetID() string { return m.id } + +func TestModifyIndexAndNamespaceIDTokenizer(t *testing.T) { + ci.Parallel(t) + + cases := []struct { + name string + obj *mockModifyIndexObject + target string + expectedToken string + expectedCmp int + }{ + { + name: "less index", + obj: newMockModifyIndexObject(12, "default", "aaa"), + target: "89.default.aaa", + expectedToken: "12.default.aaa", + expectedCmp: -1, + }, + { + name: "greater index", + obj: newMockModifyIndexObject(89, "default", "aaa"), + target: "12.default.aaa", + expectedToken: "89.default.aaa", + expectedCmp: 1, + }, + { + // index is compared numerically, not lexically: 12 < 102 + name: "numeric index (less)", + obj: newMockModifyIndexObject(12, "default", "aaa"), + target: "102.default.aaa", + expectedToken: "12.default.aaa", + expectedCmp: -1, + }, + { + name: "equal index, less namespace", + obj: newMockModifyIndexObject(12, "aaa", "x"), + target: "12.bbb.x", + expectedToken: "12.aaa.x", + expectedCmp: -1, + }, + { + name: "equal index, greater namespace", + obj: newMockModifyIndexObject(12, "bbb", "x"), + target: "12.aaa.x", + expectedToken: "12.bbb.x", + expectedCmp: 1, + }, + { + // namespace compared field-by-field, so "team" sorts before + // "team-a" (matching memdb's null-separated key order), unlike a + // whole-string compare where '.' > '-' would reverse them. + name: "dash namespace ordering", + obj: newMockModifyIndexObject(12, "team", "z"), + target: "12.team-a.a", + expectedToken: "12.team.z", + expectedCmp: -1, + }, + { + name: "equal index and namespace, less id", + obj: newMockModifyIndexObject(12, "team", "aaa"), + target: "12.team.bbb", + expectedToken: "12.team.aaa", + expectedCmp: -1, + }, + { + name: "equal index, namespace, and id", + obj: newMockModifyIndexObject(12, "team", "aaa"), + target: "12.team.aaa", + expectedToken: "12.team.aaa", + expectedCmp: 0, + }, + { + // id may contain '.'; it's the remainder after the first two splits. + name: "id with dots", + obj: newMockModifyIndexObject(12, "default", "a.b.c"), + target: "12.default.a.b.c", + expectedToken: "12.default.a.b.c", + expectedCmp: 0, + }, + { + // legacy bare-integer token (rolling upgrade): index-only compare + name: "legacy bare-integer target (equal)", + obj: newMockModifyIndexObject(12, "default", "aaa"), + target: "12", + expectedToken: "12.default.aaa", + expectedCmp: 0, + }, + { + name: "legacy bare-integer target (less)", + obj: newMockModifyIndexObject(12, "default", "aaa"), + target: "13", + expectedToken: "12.default.aaa", + expectedCmp: -1, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + fn := ModifyIndexAndNamespaceIDTokenizer[*mockModifyIndexObject](tc.target) + actualToken, actualCmp := fn(tc.obj) + must.Eq(t, tc.expectedToken, actualToken) + must.Eq(t, tc.expectedCmp, actualCmp) + }) + } +} + +func newMockModifyIndexObject(modifyIndex uint64, namespace, id string) *mockModifyIndexObject { + return &mockModifyIndexObject{ + modifyIndex: modifyIndex, + namespace: namespace, + id: id, + } +} + +type mockModifyIndexObject struct { + modifyIndex uint64 + namespace string + id string +} + +func (m *mockModifyIndexObject) GetModifyIndex() uint64 { return m.modifyIndex } +func (m *mockModifyIndexObject) GetNamespace() string { return m.namespace } +func (m *mockModifyIndexObject) GetID() string { return m.id } diff --git a/ui/app/controllers/jobs/index.js b/ui/app/controllers/jobs/index.js index 79952a1d15e..84f8c1d54a0 100644 --- a/ui/app/controllers/jobs/index.js +++ b/ui/app/controllers/jobs/index.js @@ -89,9 +89,25 @@ export default class JobsIndexController extends Controller { @tracked cursorAt; @tracked nextToken; // route sets this when new data is fetched + // Stack of the page-start cursors we've navigated through, so "prev" can step + // back without re-deriving a token. Each entry is the cursorAt that started a + // page (null for the first page). The pagination token is opaque, so we never + // do arithmetic on it. + @tracked previousTokens = []; + + // cursorFor builds the pagination cursor for a job, matching the server's + // ".." jobs/statuses page token. Only "last" has + // to synthesize a cursor; first/next/prev reuse tokens we've already seen. + cursorFor(job) { + if (!job) { + return undefined; + } + const namespace = job.belongsTo('namespace').id() || 'default'; + return `${job.modifyIndex}.${namespace}.${job.plainId}`; + } + /** - * - * @param {"prev"|"next"} page + * @param {"prev"|"next"|"first"|"last"} page */ @action async handlePageChange(page) { // reset indexes @@ -99,39 +115,34 @@ export default class JobsIndexController extends Controller { this.jobAllocsQueryIndex = 0; if (page === 'prev') { - if (!this.cursorAt) { - return; - } - // Note (and TODO:) this isn't particularly efficient! - // We're making an extra full request to get the nextToken we need, - // but actually the results of that request are the reverse order, plus one job, - // of what we actually want to show on the page! - // I should investigate whether I can use the results of this query to - // overwrite this controller's jobIDs, leverage its index, and - // restart a blocking watchJobIDs here. - let prevPageToken = await this.loadPreviousPageToken(); - // If there's no nextToken, we're at the "start" of our list and can drop the cursorAt - if (!prevPageToken.meta.nextToken) { - this.cursorAt = undefined; - } else { - // cursorAt should be the highest modifyIndex from the previous query. - // This will immediately fire the route model hook with the new cursorAt - const sortedPrevPage = prevPageToken.sortBy('modifyIndex'); - this.cursorAt = prevPageToken - ? sortedPrevPage[sortedPrevPage.length - 1]?.modifyIndex - : undefined; - } + // Step back to the previous page's cursor. Nothing to pop means we're + // returning to the first page. + const tokens = [...this.previousTokens]; + const prev = tokens.pop(); + this.previousTokens = tokens; + this.cursorAt = prev ?? undefined; } else if (page === 'next') { if (!this.nextToken) { return; } + // Remember where this page started, then advance using the opaque token + // the server handed us. + this.previousTokens = [...this.previousTokens, this.cursorAt ?? null]; this.cursorAt = this.nextToken; } else if (page === 'first') { + this.previousTokens = []; this.cursorAt = undefined; } else if (page === 'last') { - let prevPageToken = await this.loadPreviousPageToken({ last: true }); - const sortedPrevPage = prevPageToken.sortBy('modifyIndex'); - this.cursorAt = sortedPrevPage[sortedPrevPage.length - 1]?.modifyIndex; + // We don't have the last page's start cursor, so fetch the final page + // from the opposite (reverse) end and use its newest job as the cursor. + // Record the current position so "prev" returns here. + const lastPage = await this.loadLastPage(); + const sorted = lastPage.sortBy('modifyIndex'); + const boundary = sorted[sorted.length - 1]; + if (boundary) { + this.previousTokens = [...this.previousTokens, this.cursorAt ?? null]; + this.cursorAt = this.cursorFor(boundary); + } } } @@ -278,18 +289,12 @@ export default class JobsIndexController extends Controller { }); } - // Ask for the previous #page_size jobs, starting at the first job that's currently shown - // on our page, and the last one in our list should be the one we use for our - // subsequent nextToken. - async loadPreviousPageToken({ last = false } = {}) { - let next_token = +this.cursorAt + 1; - if (last) { - next_token = undefined; - } - let prevPageToken = await this.store.query( + // Fetch the final page by querying from the opposite (reverse) end. The + // newest job in that page is where the last page starts. + async loadLastPage() { + return this.store.query( 'job', { - next_token, per_page: this.pageSize, reverse: true, }, @@ -299,7 +304,6 @@ export default class JobsIndexController extends Controller { }, }, ); - return prevPageToken; } @restartableTask *watchJobIDs( @@ -601,6 +605,7 @@ export default class JobsIndexController extends Controller { @action updateFilter() { this.cursorAt = null; + this.previousTokens = []; this.filter = this.computedFilter; } diff --git a/ui/mirage/config.js b/ui/mirage/config.js index 59833f022f5..461b6310987 100644 --- a/ui/mirage/config.js +++ b/ui/mirage/config.js @@ -290,29 +290,58 @@ export default function (config) { }); } + // The jobs/statuses cursor is "..", + // compared numerically on the index then lexically on namespace and + // id (matching the server's ModifyIndexAndNamespaceIDTokenizer). + const tokenOf = (job) => + `${job.ModifyIndex}.${job.NamespaceID || 'default'}.${job.ID}`; + const parseTok = (t) => { + const s = String(t); + const i = s.indexOf('.'); + if (i < 0) return { idx: parseInt(s, 10) || 0, ns: '', id: '' }; + const rest = s.slice(i + 1); + const j = rest.indexOf('.'); + return { + idx: parseInt(s.slice(0, i), 10) || 0, + ns: j < 0 ? rest : rest.slice(0, j), + id: j < 0 ? '' : rest.slice(j + 1), + }; + }; + const cmpTok = (at, bt) => { + const a = parseTok(at); + const b = parseTok(bt); + if (a.idx !== b.idx) return a.idx - b.idx; + if (a.ns !== b.ns) return a.ns < b.ns ? -1 : 1; + if (a.id !== b.id) return a.id < b.id ? -1 : 1; + return 0; + }; + let sortedJson = filteredJson .sort((a, b) => reverse - ? a.ModifyIndex - b.ModifyIndex - : b.ModifyIndex - a.ModifyIndex, + ? cmpTok(tokenOf(a), tokenOf(b)) + : cmpTok(tokenOf(b), tokenOf(a)), ) .filter((job) => { if (namespace === '*') return true; return namespace === 'default' ? !job.NamespaceID || job.NamespaceID === 'default' : job.NamespaceID === namespace; - }) - .map((job) => filterKeys(job, 'TaskGroups', 'NamespaceID')); + }); if (nextToken) { sortedJson = sortedJson.filter((job) => reverse - ? job.ModifyIndex >= nextToken - : job.ModifyIndex <= nextToken, + ? cmpTok(tokenOf(job), nextToken) >= 0 + : cmpTok(tokenOf(job), nextToken) <= 0, ); } + sortedJson = sortedJson.map((job) => ({ + ...filterKeys(job, 'TaskGroups', 'NamespaceID'), + _cursor: tokenOf(job), + })); return sortedJson; }, - { pagination: true, tokenProperty: 'ModifyIndex' }, + { pagination: true, tokenProperty: '_cursor' }, ), ); diff --git a/ui/tests/acceptance/jobs-list-test.js b/ui/tests/acceptance/jobs-list-test.js index 9e6f0ba254a..5026b0bcfca 100644 --- a/ui/tests/acceptance/jobs-list-test.js +++ b/ui/tests/acceptance/jobs-list-test.js @@ -1036,6 +1036,53 @@ module('Acceptance | jobs list', function (hooks) { localStorage.removeItem('nomadPageSize'); }); + test('jobs that share a modify index paginate without duplicates or gaps', async function (assert) { + // Regression test for the ModifyIndex-only pagination cursor: several + // jobs can share a ModifyIndex, and the cursor must still reach each of + // them exactly once instead of repeating a page or stranding jobs. + const pageSize = 5; + const total = 12; // more than two pages, all sharing one modify index + localStorage.setItem('nomadPageSize', pageSize.toString()); + for (let i = 0; i < total; i++) { + const id = `shared-${String(i).padStart(2, '0')}`; + this.server.create('job', { + id, + name: id, + namespaceId: 'default', + modifyIndex: 1000, // identical across all of them + createAllocations: false, + shallow: true, + }); + } + + await JobsList.visit(); + + // Walk forward to the end, collecting every job we see on each page. + const seen = []; + for (let guard = 0; guard <= total; guard++) { + for (let i = 0; i < JobsList.jobs.length; i++) { + seen.push(JobsList.jobs.objectAt(i).name); + } + const next = document.querySelector('[data-test-pager="next"]'); + if (!next || next.disabled) { + break; + } + await click('[data-test-pager="next"]'); + } + + const unique = new Set(seen); + assert.strictEqual( + seen.length, + unique.size, + 'no job was shown on more than one page', + ); + assert.strictEqual( + unique.size, + total, + 'every job sharing the modify index was reachable', + ); + localStorage.removeItem('nomadPageSize'); + }); }); module('Live updates are reflected in the list', function () { test('When you have live updates enabled, the list updates when new jobs are created', async function (assert) {