hudi-agent commented on code in PR #18689:
URL: https://github.com/apache/hudi/pull/18689#discussion_r3188857639
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/sources/helpers/KinesisReadConfig.java:
##########
@@ -41,12 +42,22 @@ public class KinesisReadConfig implements Serializable {
private final String accessKey; // null if not set
private final String secretKey; // null if not set
private final KinesisSourceConfig.KinesisStartingPositionStrategy
startingPosition;
+ @Getter(AccessLevel.NONE)
private final boolean shouldAddMetaFields;
- private final boolean enableDeaggregation;
+ @Getter(AccessLevel.NONE)
Review Comment:
🤖 nit: the `@Getter(AccessLevel.NONE)` suppression here seems unnecessary —
for a primitive `boolean deaggregationEnabled` field, Lombok would
auto-generate `isDeaggregationEnabled()` exactly as written below. The
suppression + manual method is needed for `shouldAddMetaFields` (to avoid the
awkward `isShouldAddMetaFields()`), but could you drop both the annotation and
the manual method for `deaggregationEnabled` and let Lombok handle it?
<sub><i>- AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/sources/KinesisSource.java:
##########
@@ -237,7 +237,7 @@ private void fetchNextPage() {
response = client.getRecords(
GetRecordsRequest.builder()
.shardIterator(shardIteratorStr)
- .limit(Math.min(currentMaxRecords, (int) (maxTotalRecords -
totalConsumed)))
+ .limit(Math.min(currentMaxRecords,
Math.toIntExact(maxTotalRecords - totalConsumed)))
Review Comment:
🤖 If a user configures `hoodie.streamer.source.kinesis.max.events` greater
than `Integer.MAX_VALUE` (the default 5M is fine, but the type is `Long`),
`Math.toIntExact(maxTotalRecords - totalConsumed)` will throw
`ArithmeticException` on the very first call here, since `totalConsumed=0`.
Could you switch to `(int) Math.min((long) currentMaxRecords, maxTotalRecords -
totalConsumed)` instead? That avoids both silent truncation and the
ArithmeticException, since the result is always bounded by `currentMaxRecords`
≤ `Integer.MAX_VALUE`.
<sub><i>- AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
