You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fluss.apache.org by "fresh-borzoni (via GitHub)" <gi...@apache.org> on 2026/02/20 18:11:09 UTC

[PR] [TASK-332] max poll records config option [fluss-rust]

fresh-borzoni opened a new pull request, #355:
URL: https://github.com/apache/fluss-rust/pull/355

   ## Summary
   
   closes #332 
   
   Introduce configurable `scanner.log.max-poll-records` as in Java for all 3 clients.


-- 
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: issues-unsubscribe@fluss.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [TASK-332] max poll records config option [fluss-rust]

Posted by "leekeiabstraction (via GitHub)" <gi...@apache.org>.
leekeiabstraction commented on code in PR #355:
URL: https://github.com/apache/fluss-rust/pull/355#discussion_r2834665232


##########
bindings/python/src/config.rs:
##########
@@ -70,6 +70,11 @@ impl Config {
                             config.remote_file_download_thread_num = num;
                         }
                     }
+                    "scanner.log.max-poll-records" => {
+                        if let Ok(num) = value.parse::<usize>() {
+                            config.scanner_log_max_poll_records = num;

Review Comment:
   It seems like invalid values are silently dropped here. We should ideally error and fail fast instead of dropping the value and causing unexpected behaviour.



##########
website/docs/user-guide/cpp/api-reference.md:
##########
@@ -24,6 +24,7 @@ Complete API reference for the Fluss C++ client.
 | `writer_batch_size`               | `int32_t`     | `2097152` (2 MB)     | Batch size for writes in bytes                                  |
 | `scanner_remote_log_prefetch_num` | `size_t`      | `4`                  | Number of remote log segments to prefetch                       |
 | `remote_file_download_thread_num` | `size_t`      | `3`                  | Number of threads for remote log downloads                      |
+| `scanner_log_max_poll_records`    | `size_t`      | `500`                | Maximum number of records returned in a single Poll()           |

Review Comment:
   Should we also have dictionary / hash map style configuration for cpp?



-- 
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: issues-unsubscribe@fluss.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [TASK-332] max poll records config option [fluss-rust]

Posted by "fresh-borzoni (via GitHub)" <gi...@apache.org>.
fresh-borzoni commented on code in PR #355:
URL: https://github.com/apache/fluss-rust/pull/355#discussion_r2834914552


##########
website/docs/user-guide/cpp/api-reference.md:
##########
@@ -24,6 +24,7 @@ Complete API reference for the Fluss C++ client.
 | `writer_batch_size`               | `int32_t`     | `2097152` (2 MB)     | Batch size for writes in bytes                                  |
 | `scanner_remote_log_prefetch_num` | `size_t`      | `4`                  | Number of remote log segments to prefetch                       |
 | `remote_file_download_thread_num` | `size_t`      | `3`                  | Number of threads for remote log downloads                      |
+| `scanner_log_max_poll_records`    | `size_t`      | `500`                | Maximum number of records returned in a single Poll()           |

Review Comment:
   typed structs is more idiomatic for C++, python uses dict just bc it's more pythonic way. Not that we load configs dynamically anywhere.



-- 
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: issues-unsubscribe@fluss.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [TASK-332] max poll records config option [fluss-rust]

Posted by "fresh-borzoni (via GitHub)" <gi...@apache.org>.
fresh-borzoni commented on PR #355:
URL: https://github.com/apache/fluss-rust/pull/355#issuecomment-3936396785

   @leekeiabstraction PTAL 🙏 


-- 
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: issues-unsubscribe@fluss.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [TASK-332] max poll records config option [fluss-rust]

Posted by "luoyuxia (via GitHub)" <gi...@apache.org>.
luoyuxia merged PR #355:
URL: https://github.com/apache/fluss-rust/pull/355


-- 
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: issues-unsubscribe@fluss.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [TASK-332] max poll records config option [fluss-rust]

Posted by "fresh-borzoni (via GitHub)" <gi...@apache.org>.
fresh-borzoni commented on PR #355:
URL: https://github.com/apache/fluss-rust/pull/355#issuecomment-3936941443

   @leekeiabstraction Ty for the review.
   Addressed comment. PTAL 🙏 


-- 
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: issues-unsubscribe@fluss.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [TASK-332] max poll records config option [fluss-rust]

Posted by "fresh-borzoni (via GitHub)" <gi...@apache.org>.
fresh-borzoni commented on code in PR #355:
URL: https://github.com/apache/fluss-rust/pull/355#discussion_r2834902492


##########
bindings/python/src/config.rs:
##########
@@ -70,6 +70,11 @@ impl Config {
                             config.remote_file_download_thread_num = num;
                         }
                     }
+                    "scanner.log.max-poll-records" => {
+                        if let Ok(num) = value.parse::<usize>() {
+                            config.scanner_log_max_poll_records = num;

Review Comment:
   yeah, it's better to fix for the other configs, since we are here. Good catch 👍 



-- 
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: issues-unsubscribe@fluss.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org