Copilot commented on code in PR #60141:
URL: https://github.com/apache/doris/pull/60141#discussion_r2715765097
##########
fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java:
##########
@@ -1224,7 +1224,7 @@ public void checkQuerySlotCount(String slotCnt) {
// 4096 minus 16 + 16 bytes padding that in padding pod array
Review Comment:
The inline comment above `batchSize` still describes the old 4096/4064
sizing rationale, but the default is now 8160. Please update the comment to
reflect the new intended sizing (e.g., based on 8192 and any header/padding
bytes) so future readers don’t assume the value is tied to 4KB.
```suggestion
// 8KB page (8192 bytes) minus 32 bytes reserved for
header/metadata/padding
```
##########
fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java:
##########
@@ -1210,7 +1210,7 @@ public void checkQuerySlotCount(String slotCnt) {
public String lcTimeNames = "en_US";
@VariableMgr.VarAttr(name = PARALLEL_EXCHANGE_INSTANCE_NUM)
- public int exchangeInstanceParallel = 100;
+ public int exchangeInstanceParallel = 256;
Review Comment:
These changes alter user-visible default session settings (parallel exchange
instances and batch size), but there doesn’t appear to be a unit test asserting
the default values. Consider adding/updating a FE unit test (e.g., in
`VariableMgrTest` or `SessionVariablesTest`) to assert the new defaults so
accidental regressions are caught.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
