rpuch commented on code in PR #1246:
URL: https://github.com/apache/ignite-3/pull/1246#discussion_r1006501021
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/message/SnapshotTxDataRequest.java:
##########
@@ -25,4 +25,10 @@
*/
@Transferable(TableMessageGroup.SNAPSHOT_TX_DATA_REQUEST)
public interface SnapshotTxDataRequest extends SnapshotRequestMessage {
+ /**
+ * Returns maximum number of transactions that should be sent in response.
+ *
+ * @return Maximum number of transactions that should be sent in response.
+ */
+ int maxTransactionsInBatch();
Review Comment:
Removing these would require us to hardcode batch sizes on the sender size.
Keeping them allows us to hardcode these on the receiver size, or, if needed,
define them by a property or even compute adaptively. You don't like
adaptiveness, but it's cool to have an open door if we need it.
The 'overengineering' is tiny, it's just having 2 additional parameters.
It's not even worth the words we spend here.
Testing is important. Testing the paging logic would become pretty ugly: the
tests would have to know the batch sizes and craft rows/tx data specifically to
match/exceed these limits instead of more natural direct control. Do we want to
pay this price for avoiding to add 2 parameters (that will probably be added in
the future anyway)?
--
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]
