-
Notifications
You must be signed in to change notification settings - Fork 2.1k
[Improve][engine-server] engine-server and seantunnel-ui support remote paginated queries #9951
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
| int start = (page - 1) * rows; | ||
| JsonArray paginatedArray = new JsonArray(); | ||
| jsonArray | ||
| .values() | ||
| .subList(start, Math.min(start + rows, total)) | ||
| .forEach( | ||
| t -> { | ||
| paginatedArray.add(t); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validation is needed. If start > total or page < 0, an error may occur, causing the program to malfunction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validation has been added, and an exception will be thrown if the conditions are not met.
|
cc @hawk9821 |
| restApiRequestHttp( | ||
| "http://localhost:" + HTTP_PORT + "/finished-jobs?page=1&rows=5", | ||
| response -> { | ||
| JsonObject resultJson = (JsonObject) Json.parse(response); | ||
| Assertions.assertTrue( | ||
| resultJson.get("data") != null && resultJson.get("total") != null); | ||
| }); | ||
| // no pagination test | ||
| restApiRequestHttp( | ||
| "http://localhost:" + HTTP_PORT + "/finished-jobs", | ||
| response -> { | ||
| JsonArray resultJson = (JsonArray) Json.parse(response); | ||
| Assertions.assertTrue(resultJson != null); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current checking logic is a bit weak. Can we verify that the amount of data returned and the results meet expectations by modifying different parameters in the test case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented validation checks for API return row counts and added unit tests to verify behavior when page numbers exceed available data.
| private final String pageParam = "page"; | ||
| private final String rowsParam = "rows"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The API parameters in the docs rest-api-v2.md have been updated.
55d9800 to
9d78efc
Compare
96879ff to
97d153f
Compare
| nodeEngine.getHazelcastInstance().getMap(Constant.IMAP_RUNNING_JOB_INFO); | ||
| return values.entrySet().stream() | ||
| .map(jobInfoEntry -> convertToJson(jobInfoEntry.getValue(), jobInfoEntry.getKey())) | ||
| .sorted( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JobId is ordered, I think it can be sorted by jobId, compared to higher efficiency.
return values.entrySet().stream()
.sorted(Map.Entry.comparingByKey(Comparator.reverseOrder()))
.map(jobInfoEntry -> convertToJson(jobInfoEntry.getValue(), jobInfoEntry.getKey()))
.collect(JsonArray::new, JsonArray::add, JsonArray::add);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JobId is manually passed in and may not guarantee order, so it has been changed to use the InitializationTimestamp of the JobInfo for sorting.
97d153f to
6580495
Compare
Purpose of this pull request
Objective: Enable pagination for querying finished and running tasks in the engine-ui interface.
Rationale: When the Seatunnel cluster handles numerous tasks,For example, when exceeding 2,000 tasks, fetching all tasks simultaneously can cause:
The effect after implementing pagination is as follows:


Does this PR introduce any user-facing change?
No
How was this patch tested?
Check list
New License Guide