Re: [PR] [FLINK-34954][core] Kryo Input bug fix [flink]

[via GitHub]

dmvk commented on PR #24586:
URL: https://github.com/apache/flink/pull/24586#issuecomment-2122091374

   Why is this not covered with a regression test? That feels rather scary in 
such a critical part of the stack.


-- 
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-unsubscr...@flink.apache.org

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

Re: [PR] [FLINK-34954][core] Kryo Input bug fix [flink]

[via GitHub]

davidradl commented on code in PR #24586:
URL: https://github.com/apache/flink/pull/24586#discussion_r1582776111


##
flink-core/src/main/java/org/apache/flink/api/java/typeutils/runtime/NoFetchingInput.java:
##
@@ -73,17 +73,14 @@ protected int require(int required) throws KryoException {
 position = 0;
 int bytesRead = 0;
 int count;
-while (true) {
+while (bytesRead < required) {
 count = fill(buffer, bytesRead, required - bytesRead);

Review Comment:
   @dannycranmer I saw this linked to in slack - I wondered whether we could 
have junits for the required case and higher than required? Maybe as a follow 
on issue / pr ? I could code if you were willing to merge for me? 



-- 
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-unsubscr...@flink.apache.org

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

Re: [PR] [FLINK-34954][core] Kryo Input bug fix [flink]

[via GitHub]

dannycranmer merged PR #24586:
URL: https://github.com/apache/flink/pull/24586


-- 
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-unsubscr...@flink.apache.org

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

Re: [PR] [FLINK-34954][core] Kryo Input bug fix [flink]

[via GitHub]

qinghui-xu commented on PR #24586:
URL: https://github.com/apache/flink/pull/24586#issuecomment-2039567737

   Hello team,
   Could I get a review for this, please?


-- 
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-unsubscr...@flink.apache.org

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

Re: [PR] [FLINK-34954][core] Kryo Input bug fix [flink]

[via GitHub]

flinkbot commented on PR #24586:
URL: https://github.com/apache/flink/pull/24586#issuecomment-2024784567

   
   ## CI report:
   
   * a37e562b54ed9ad4b9290f2f999542ea9104c65f UNKNOWN
   
   
   Bot commands
 The @flinkbot bot supports the following commands:
   
- `@flinkbot run azure` re-run the last Azure build
   


-- 
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-unsubscr...@flink.apache.org

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