abh1sar commented on code in PR #9096:
URL: https://github.com/apache/cloudstack/pull/9096#discussion_r1609315083
##########
server/src/main/java/com/cloud/network/NetworkServiceImpl.java:
##########
@@ -2267,7 +2267,7 @@ public Pair<List<? extends Network>, Integer>
searchForNetworks(ListNetworksCmd
isRecursive = true;
}
- Filter searchFilter = new Filter(NetworkVO.class, "id", false, null,
null);
+ Filter searchFilter = new Filter(NetworkVO.class, "id", false,
cmd.getStartIndex(), cmd.getPageSizeVal());
Review Comment:
That's a very good point.
The issue here is that we are concatenating results from multiple queries (
lest's say Q1, Q2, Q3). Things **kind of** work for the first page. We can
adjust pageSize of a query depending on how many results are already returned
for other queries.
But if we want to display the second page, it is hard to say which
startIndex should be used for Q2. As it depends on how many results were
displayed for Q2 on page 1. Which in turn depends on how many Q1 results were
there on Page 1. For example, if page 1 had 15 results for Q1 and 5 results for
Q2, then page2 Q2 startIndex should be 6.
So, to fix this I think the filter should always have `startIndex` as `0`
and `pageSize` should be `cmd.getPageSizeVal() * (cmd.getStartIndex() + 1)`
`Filter searchFilter = new Filter(NetworkVO.class, "id", false, 0,
(cmd.getStartIndex() + 1) * cmd.getPageSizeVal());`
This way we have info on all previous page results and when pagination is
applied at the end of the method, the correct result will be returned.
We are losing some optimization if startIndex is higher but it is still
better than reading all rows.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
