You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/01/19 12:52:53 UTC
[GitHub] [flink] killxdcj opened a new pull request #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
killxdcj opened a new pull request #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903
## What is the purpose of the change
The ResettableIterator has a data overflow problem if the number of elements in a single input exceeds Integer.MAX_VALUE. This pull request intend to fix this problem.
## Brief change log
- *Fix Integer overflow in ResettableIterator*
## Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): no
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: no
- The serializers: no
- The runtime per-record code paths (performance sensitive): no
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: no
- The S3 file system connector: no
## Documentation
- Does this pull request introduce a new feature? no
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] killxdcj commented on a change in pull request #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
Posted by GitBox <gi...@apache.org>.
killxdcj commented on a change in pull request #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903#discussion_r368414875
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/operators/resettable/SpillingResettableMutableObjectIterator.java
##########
@@ -204,8 +204,16 @@ public void consumeAndCacheRemainingData() throws IOException {
} catch (IOException e) {
throw new RuntimeException("SpillingIterator: Error writing element to buffer.", e);
}
- this.elementCount++;
+ increaseElementCount();
}
}
}
+
+ private void increaseElementCount() {
Review comment:
You can think of it this way, there may be many such scenes in reality, and you need to check the overflow, so whether all the places to check the overflow need to use the same code.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] killxdcj commented on issue #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
Posted by GitBox <gi...@apache.org>.
killxdcj commented on issue #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903#issuecomment-576492042
@flinkbot run travis
@flinkbot run azure
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] pnowojski commented on issue #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
Posted by GitBox <gi...@apache.org>.
pnowojski commented on issue #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903#issuecomment-576581362
Thanks, failure is unrelated: https://issues.apache.org/jira/browse/FLINK-15247 merging.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903#issuecomment-576004518
<!--
Meta data
Hash:42bc28f0d52652f76118922a27ca1f82c3b9030e Status:UNKNOWN URL:TBD TriggerType:PUSH TriggerID:42bc28f0d52652f76118922a27ca1f82c3b9030e
Hash:4289c3910b39b13938aafc1337cc9c127e46fedd Status:UNKNOWN URL:TBD TriggerType:PUSH TriggerID:4289c3910b39b13938aafc1337cc9c127e46fedd
-->
## CI report:
* 42bc28f0d52652f76118922a27ca1f82c3b9030e UNKNOWN
* 4289c3910b39b13938aafc1337cc9c127e46fedd UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] pnowojski merged pull request #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
Posted by GitBox <gi...@apache.org>.
pnowojski merged pull request #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903#issuecomment-576004518
<!--
Meta data
Hash:42bc28f0d52652f76118922a27ca1f82c3b9030e Status:UNKNOWN URL:TBD TriggerType:PUSH TriggerID:42bc28f0d52652f76118922a27ca1f82c3b9030e
Hash:4289c3910b39b13938aafc1337cc9c127e46fedd Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/145110829 TriggerType:PUSH TriggerID:4289c3910b39b13938aafc1337cc9c127e46fedd
Hash:4289c3910b39b13938aafc1337cc9c127e46fedd Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4478 TriggerType:PUSH TriggerID:4289c3910b39b13938aafc1337cc9c127e46fedd
-->
## CI report:
* 42bc28f0d52652f76118922a27ca1f82c3b9030e UNKNOWN
* 4289c3910b39b13938aafc1337cc9c127e46fedd Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/145110829) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4478)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] AHeise commented on a change in pull request #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
Posted by GitBox <gi...@apache.org>.
AHeise commented on a change in pull request #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903#discussion_r368541850
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/operators/resettable/SpillingResettableMutableObjectIterator.java
##########
@@ -204,8 +204,16 @@ public void consumeAndCacheRemainingData() throws IOException {
} catch (IOException e) {
throw new RuntimeException("SpillingIterator: Error writing element to buffer.", e);
}
- this.elementCount++;
+ increaseElementCount();
}
}
}
+
+ private void increaseElementCount() {
Review comment:
Extracting that logic to some `NumericUtil#safeIncrement` in `flink-core` could be a viable option.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot commented on issue #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
Posted by GitBox <gi...@apache.org>.
flinkbot commented on issue #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903#issuecomment-576004518
<!--
Meta data
Hash:42bc28f0d52652f76118922a27ca1f82c3b9030e Status:UNKNOWN URL:TBD TriggerType:PUSH TriggerID:42bc28f0d52652f76118922a27ca1f82c3b9030e
-->
## CI report:
* 42bc28f0d52652f76118922a27ca1f82c3b9030e UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903#issuecomment-576004518
<!--
Meta data
Hash:42bc28f0d52652f76118922a27ca1f82c3b9030e Status:UNKNOWN URL:TBD TriggerType:PUSH TriggerID:42bc28f0d52652f76118922a27ca1f82c3b9030e
Hash:4289c3910b39b13938aafc1337cc9c127e46fedd Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/145110829 TriggerType:PUSH TriggerID:4289c3910b39b13938aafc1337cc9c127e46fedd
Hash:4289c3910b39b13938aafc1337cc9c127e46fedd Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4478 TriggerType:PUSH TriggerID:4289c3910b39b13938aafc1337cc9c127e46fedd
Hash:4289c3910b39b13938aafc1337cc9c127e46fedd Status:CANCELED URL:https://travis-ci.com/flink-ci/flink/builds/145110829 TriggerType:MANUAL TriggerID:576492042
Hash:4289c3910b39b13938aafc1337cc9c127e46fedd Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4478 TriggerType:MANUAL TriggerID:576492042
Hash:d607f61ab374d5ee630dfc72686a4e65d2fc8548 Status:PENDING URL:https://travis-ci.com/flink-ci/flink/builds/145301968 TriggerType:PUSH TriggerID:d607f61ab374d5ee630dfc72686a4e65d2fc8548
Hash:d607f61ab374d5ee630dfc72686a4e65d2fc8548 Status:PENDING URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4518 TriggerType:PUSH TriggerID:d607f61ab374d5ee630dfc72686a4e65d2fc8548
-->
## CI report:
* 42bc28f0d52652f76118922a27ca1f82c3b9030e UNKNOWN
* 4289c3910b39b13938aafc1337cc9c127e46fedd Travis: [CANCELED](https://travis-ci.com/flink-ci/flink/builds/145110829) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4478)
* d607f61ab374d5ee630dfc72686a4e65d2fc8548 Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/145301968) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4518)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] killxdcj commented on a change in pull request #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
Posted by GitBox <gi...@apache.org>.
killxdcj commented on a change in pull request #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903#discussion_r368415858
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/operators/resettable/SpillingResettableMutableObjectIterator.java
##########
@@ -204,8 +204,16 @@ public void consumeAndCacheRemainingData() throws IOException {
} catch (IOException e) {
throw new RuntimeException("SpillingIterator: Error writing element to buffer.", e);
}
- this.elementCount++;
+ increaseElementCount();
}
}
}
+
+ private void increaseElementCount() {
Review comment:
Maybe you can think of it this way, there may be many such scenes in reality, and you need to check the overflow, so whether all the places to check the overflow need to use the same code.
And the point is that they are two separate components, and there is no need to reuse them for reuse.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] killxdcj commented on a change in pull request #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
Posted by GitBox <gi...@apache.org>.
killxdcj commented on a change in pull request #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903#discussion_r368352052
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/operators/resettable/SpillingResettableMutableObjectIterator.java
##########
@@ -204,8 +204,16 @@ public void consumeAndCacheRemainingData() throws IOException {
} catch (IOException e) {
throw new RuntimeException("SpillingIterator: Error writing element to buffer.", e);
}
- this.elementCount++;
+ increaseElementCount();
}
}
}
+
+ private void increaseElementCount() {
Review comment:
Hi, @KarmaGYZ Thanks for your comment.
SpillingResettableIterator and SpillingResettableMutableObjectIterator both implemented ResettableIterator, but they are two separate components. Although the logic of adding elementCount is somewhat similar, it is actually related to their respective implementation logic, so i think there is no need to share it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot commented on issue #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
Posted by GitBox <gi...@apache.org>.
flinkbot commented on issue #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903#issuecomment-576001970
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.
## Automated Checks
Last check on commit 42bc28f0d52652f76118922a27ca1f82c3b9030e (Sun Jan 19 12:56:35 UTC 2020)
**Warnings:**
* No documentation files were touched! Remember to keep the Flink docs up to date!
<sub>Mention the bot in a comment to re-run the automated checks.</sub>
## Review Progress
* ❓ 1. The [description] looks good.
* ❓ 2. There is [consensus] that the contribution should go into to Flink.
* ❓ 3. Needs [attention] from.
* ❓ 4. The change fits into the overall [architecture].
* ❓ 5. Overall code [quality] is good.
Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
- `@flinkbot approve all` to approve all aspects
- `@flinkbot approve-until architecture` to approve everything until `architecture`
- `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
- `@flinkbot disapprove architecture` to remove an approval you gave earlier
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903#issuecomment-576004518
<!--
Meta data
Hash:42bc28f0d52652f76118922a27ca1f82c3b9030e Status:UNKNOWN URL:TBD TriggerType:PUSH TriggerID:42bc28f0d52652f76118922a27ca1f82c3b9030e
Hash:4289c3910b39b13938aafc1337cc9c127e46fedd Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/145110829 TriggerType:PUSH TriggerID:4289c3910b39b13938aafc1337cc9c127e46fedd
Hash:4289c3910b39b13938aafc1337cc9c127e46fedd Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4478 TriggerType:PUSH TriggerID:4289c3910b39b13938aafc1337cc9c127e46fedd
Hash:4289c3910b39b13938aafc1337cc9c127e46fedd Status:CANCELED URL:https://travis-ci.com/flink-ci/flink/builds/145110829 TriggerType:MANUAL TriggerID:576492042
Hash:4289c3910b39b13938aafc1337cc9c127e46fedd Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4478 TriggerType:MANUAL TriggerID:576492042
Hash:d607f61ab374d5ee630dfc72686a4e65d2fc8548 Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/145301968 TriggerType:PUSH TriggerID:d607f61ab374d5ee630dfc72686a4e65d2fc8548
Hash:d607f61ab374d5ee630dfc72686a4e65d2fc8548 Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4518 TriggerType:PUSH TriggerID:d607f61ab374d5ee630dfc72686a4e65d2fc8548
-->
## CI report:
* 42bc28f0d52652f76118922a27ca1f82c3b9030e UNKNOWN
* 4289c3910b39b13938aafc1337cc9c127e46fedd Travis: [CANCELED](https://travis-ci.com/flink-ci/flink/builds/145110829) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4478)
* d607f61ab374d5ee630dfc72686a4e65d2fc8548 Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/145301968) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4518)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903#issuecomment-576004518
<!--
Meta data
Hash:42bc28f0d52652f76118922a27ca1f82c3b9030e Status:UNKNOWN URL:TBD TriggerType:PUSH TriggerID:42bc28f0d52652f76118922a27ca1f82c3b9030e
Hash:4289c3910b39b13938aafc1337cc9c127e46fedd Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/145110829 TriggerType:PUSH TriggerID:4289c3910b39b13938aafc1337cc9c127e46fedd
Hash:4289c3910b39b13938aafc1337cc9c127e46fedd Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4478 TriggerType:PUSH TriggerID:4289c3910b39b13938aafc1337cc9c127e46fedd
Hash:4289c3910b39b13938aafc1337cc9c127e46fedd Status:CANCELED URL:https://travis-ci.com/flink-ci/flink/builds/145110829 TriggerType:MANUAL TriggerID:576492042
Hash:4289c3910b39b13938aafc1337cc9c127e46fedd Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4478 TriggerType:MANUAL TriggerID:576492042
Hash:d607f61ab374d5ee630dfc72686a4e65d2fc8548 Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/145301968 TriggerType:PUSH TriggerID:d607f61ab374d5ee630dfc72686a4e65d2fc8548
Hash:d607f61ab374d5ee630dfc72686a4e65d2fc8548 Status:PENDING URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4518 TriggerType:PUSH TriggerID:d607f61ab374d5ee630dfc72686a4e65d2fc8548
-->
## CI report:
* 42bc28f0d52652f76118922a27ca1f82c3b9030e UNKNOWN
* 4289c3910b39b13938aafc1337cc9c127e46fedd Travis: [CANCELED](https://travis-ci.com/flink-ci/flink/builds/145110829) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4478)
* d607f61ab374d5ee630dfc72686a4e65d2fc8548 Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/145301968) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4518)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] AHeise commented on issue #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
Posted by GitBox <gi...@apache.org>.
AHeise commented on issue #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903#issuecomment-576579125
LGTM.
Btw "The runtime per-record code paths (performance sensitive): no" should probably be a yes next time.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] AHeise commented on a change in pull request #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
Posted by GitBox <gi...@apache.org>.
AHeise commented on a change in pull request #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903#discussion_r368540191
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/operators/resettable/SpillingResettableIterator.java
##########
@@ -189,4 +189,12 @@ public void remove() {
return memory;
}
}
+
+ private void increaseElementCount() {
Review comment:
This method is a bit overengineered: even if we are doing nothing than just incrementing the long value, it takes 60 years on a 4 GHz CPU to reach the overflow in java.
Nevertheless, I expect the branch prediction to also figure that out and not cause any performance degradation.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] KarmaGYZ commented on a change in pull request #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
Posted by GitBox <gi...@apache.org>.
KarmaGYZ commented on a change in pull request #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903#discussion_r368346480
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/operators/resettable/SpillingResettableMutableObjectIterator.java
##########
@@ -204,8 +204,16 @@ public void consumeAndCacheRemainingData() throws IOException {
} catch (IOException e) {
throw new RuntimeException("SpillingIterator: Error writing element to buffer.", e);
}
- this.elementCount++;
+ increaseElementCount();
}
}
}
+
+ private void increaseElementCount() {
Review comment:
We should avoid code duplication. Maybe it's better to add a utility function.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903#issuecomment-576004518
<!--
Meta data
Hash:42bc28f0d52652f76118922a27ca1f82c3b9030e Status:UNKNOWN URL:TBD TriggerType:PUSH TriggerID:42bc28f0d52652f76118922a27ca1f82c3b9030e
Hash:4289c3910b39b13938aafc1337cc9c127e46fedd Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/145110829 TriggerType:PUSH TriggerID:4289c3910b39b13938aafc1337cc9c127e46fedd
Hash:4289c3910b39b13938aafc1337cc9c127e46fedd Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4478 TriggerType:PUSH TriggerID:4289c3910b39b13938aafc1337cc9c127e46fedd
Hash:4289c3910b39b13938aafc1337cc9c127e46fedd Status:PENDING URL:https://travis-ci.com/flink-ci/flink/builds/145110829 TriggerType:MANUAL TriggerID:576492042
Hash:d607f61ab374d5ee630dfc72686a4e65d2fc8548 Status:UNKNOWN URL:TBD TriggerType:PUSH TriggerID:d607f61ab374d5ee630dfc72686a4e65d2fc8548
-->
## CI report:
* 42bc28f0d52652f76118922a27ca1f82c3b9030e UNKNOWN
* 4289c3910b39b13938aafc1337cc9c127e46fedd Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/145110829) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4478)
* d607f61ab374d5ee630dfc72686a4e65d2fc8548 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] killxdcj commented on a change in pull request #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
Posted by GitBox <gi...@apache.org>.
killxdcj commented on a change in pull request #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903#discussion_r368414875
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/operators/resettable/SpillingResettableMutableObjectIterator.java
##########
@@ -204,8 +204,16 @@ public void consumeAndCacheRemainingData() throws IOException {
} catch (IOException e) {
throw new RuntimeException("SpillingIterator: Error writing element to buffer.", e);
}
- this.elementCount++;
+ increaseElementCount();
}
}
}
+
+ private void increaseElementCount() {
Review comment:
You can think of it this way, there may be many such scenes in reality, and you need to check the overflow, so whether all the places to check the overflow need to use the same code.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903#issuecomment-576004518
<!--
Meta data
Hash:42bc28f0d52652f76118922a27ca1f82c3b9030e Status:UNKNOWN URL:TBD TriggerType:PUSH TriggerID:42bc28f0d52652f76118922a27ca1f82c3b9030e
Hash:4289c3910b39b13938aafc1337cc9c127e46fedd Status:PENDING URL:https://travis-ci.com/flink-ci/flink/builds/145110829 TriggerType:PUSH TriggerID:4289c3910b39b13938aafc1337cc9c127e46fedd
Hash:4289c3910b39b13938aafc1337cc9c127e46fedd Status:PENDING URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4478 TriggerType:PUSH TriggerID:4289c3910b39b13938aafc1337cc9c127e46fedd
-->
## CI report:
* 42bc28f0d52652f76118922a27ca1f82c3b9030e UNKNOWN
* 4289c3910b39b13938aafc1337cc9c127e46fedd Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/145110829) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4478)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] KarmaGYZ commented on a change in pull request #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
Posted by GitBox <gi...@apache.org>.
KarmaGYZ commented on a change in pull request #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903#discussion_r368360044
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/operators/resettable/SpillingResettableMutableObjectIterator.java
##########
@@ -204,8 +204,16 @@ public void consumeAndCacheRemainingData() throws IOException {
} catch (IOException e) {
throw new RuntimeException("SpillingIterator: Error writing element to buffer.", e);
}
- this.elementCount++;
+ increaseElementCount();
}
}
}
+
+ private void increaseElementCount() {
Review comment:
I don't think so. The logic of `increaseElementCount` in two classes is both check the numerical overflow and increase it. Am I understand it correctly?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] killxdcj commented on a change in pull request #10903:
[FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
Posted by GitBox <gi...@apache.org>.
killxdcj commented on a change in pull request #10903: [FLINK-15549][API/DataSet] Fix Integer overflow in ResettableIterator*.
URL: https://github.com/apache/flink/pull/10903#discussion_r368786540
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/operators/resettable/SpillingResettableIterator.java
##########
@@ -189,4 +189,12 @@ public void remove() {
return memory;
}
}
+
+ private void increaseElementCount() {
Review comment:
I agree with you, it was realy a bit overengineered, so I removed the logic of checking long overflow.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services