[ISSUE #5078] set default pageSize to 1000 for getting all the subscribers in other server node. #5424
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
horizonzy
reviewed
Apr 20, 2021
| int pageNo = NumberUtils.toInt(WebUtils.required(request, "pageNo")); | ||
| int pageSize = NumberUtils.toInt(WebUtils.required(request, "pageSize")); | ||
| int pageNo = NumberUtils.toInt(WebUtils.optional(request, "pageNo", "1")); | ||
| int pageSize = NumberUtils.toInt(WebUtils.optional(request, "pageSize", "10000")); |
There was a problem hiding this comment.
感觉在这里用缺省值开头太大了，这个是open api，用户可以直接使用，建议还是通过在请求时设置参数。
There was a problem hiding this comment.
可以，我待会换种方式，但是暂时还是建议通过大数值这种方式修复这个问题。
There was a problem hiding this comment.
前端传递的代码是有pageSize和pageNo的，我觉得可以暂时先透传到其他server端。
之后再修改和优化查询逻辑。
缺醒值可以有，但是必须小。
There was a problem hiding this comment.
那我暂时把缺省值将为1000，这个基本就是查所有了。
KomachiSion
requested changes
Apr 21, 2021
| int pageNo = NumberUtils.toInt(WebUtils.required(request, "pageNo")); | ||
| int pageSize = NumberUtils.toInt(WebUtils.required(request, "pageSize")); | ||
| int pageNo = NumberUtils.toInt(WebUtils.optional(request, "pageNo", "1")); | ||
| int pageSize = NumberUtils.toInt(WebUtils.optional(request, "pageSize", "10000")); |
There was a problem hiding this comment.
前端传递的代码是有pageSize和pageNo的，我觉得可以暂时先透传到其他server端。
之后再修改和优化查询逻辑。
缺醒值可以有，但是必须小。
KomachiSion
requested changes
Apr 22, 2021
|
好的，我待会调整一下，疏忽了这个问题。 |
KomachiSion
approved these changes
Apr 23, 2021
KomachiSion
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please do not create a Pull Request without creating an issue first.
What is the purpose of the change
Fixes #5078
由于每台机器的subscribers的数量都可能超过pageSize，所以暂时没有使用查本地再去查远程服务的方式。因为在做翻页的时候，必须要告诉后端已经查询了哪些机器，这样才能确认这一次查询需要查询那些机器，这样复杂度会很高，改动范围会比较大。所以暂时考虑用这种方式修复此问题。
Brief changelog
XX
Verifying this change
XXXX
Follow this checklist to help us incorporate your contribution quickly and easily:
[ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.mvn -B clean package apache-rat:check findbugs:findbugs -Dmaven.test.skip=trueto make sure basic checks pass. Runmvn clean install -DskipITsto make sure unit-test pass. Runmvn clean test-compile failsafe:integration-testto make sure integration-test pass.