Copilot commented on code in PR #3490:
URL: https://github.com/apache/iceberg-rust/pull/3490#discussion_r4321329437
##########
.asf.yaml:
##########
@@ -95,8 +95,8 @@ github:
parameters:
check_response_timeout_minutes: 360
grouping_strategy: ALLGREEN
- max_entries_to_build: 1
- max_entries_to_merge: 1
+ max_entries_to_build: 3
+ max_entries_to_merge: 3
Review Comment:
The choice of `3` here is a policy/magic-number that will likely need future
tuning. Add a short inline comment explaining why 3 was selected (e.g., based
on observed runner wait times / desired throughput) and what signal would
justify adjusting it up/down. This helps future maintainers avoid guesswork
when revisiting queue performance.
##########
.asf.yaml:
##########
@@ -95,8 +95,8 @@ github:
parameters:
check_response_timeout_minutes: 360
grouping_strategy: ALLGREEN
- max_entries_to_build: 1
- max_entries_to_merge: 1
+ max_entries_to_build: 3
+ max_entries_to_merge: 3
merge_method: SQUASH
Review Comment:
`max_entries_to_merge: 3` increases how many entries can land back-to-back
without revalidating the base branch between each individual merge (depending
on how the ASF merge queue implementation batches merges under `ALLGREEN`).
Consider keeping `max_entries_to_build` at 3 for throughput, but leaving
`max_entries_to_merge` at 1 to preserve more conservative merge serialization
(lower risk of rapid consecutive merges causing follow-on breakage/conflicts).
If you keep `max_entries_to_merge: 3`, it would be good to document the
expected batching/merge semantics and the operational trade-off being made.
--
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]
