You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "bjornarjatten (via GitHub)" <gi...@apache.org> on 2025/03/20 22:55:04 UTC
[PR] [hotfix] change bytesReceivedPerInterface to AtomicLongArray for thread safety [flink]
bjornarjatten opened a new pull request, #26332:
URL: https://github.com/apache/flink/pull/26332
## What is the purpose of the change
This pull request improves thread safety in the SystemResourcesCounter class by replacing the bytesReceivedPerInterface array with an AtomicLongArray.
Previously, bytesReceivedPerInterface was a plain long array, meaning concurrent updates from multiple threads could lead to data inconsistencies or race conditions. By using AtomicLongArray, we ensure that updates are performed atomically, preventing potential visibility and synchronization issues.
## Brief change log
- Replaced long[] bytesReceivedPerInterface with AtomicLongArray bytesReceivedPerInterface for improved thread safety.
- Updated the initialization of bytesReceivedPerInterface to use AtomicLongArray.
- Replaced direct array assignments with atomic set() and get() calls.
## 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)? Potentially, as it changes how network usage data is stored and updated.
- Anything that affects deployment or recovery (JobManager, Checkpointing, Kubernetes/Yarn, ZooKeeper)? No
- The S3 file system connector? No
## Documentation
- Does this pull request introduce a new feature? No
- If yes, how is the feature documented? Not applicable
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [hotfix] change bytesReceivedPerInterface to AtomicLongArray for thread safety [flink]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #26332:
URL: https://github.com/apache/flink/pull/26332#discussion_r2007356472
##########
flink-runtime/src/main/java/org/apache/flink/runtime/metrics/util/SystemResourcesCounter.java:
##########
@@ -269,14 +269,14 @@ private void calculateNetworkUsage(List<NetworkIF> networkIFs) {
receiveRatePerInterface.set(
i,
- (networkIF.getBytesRecv() - bytesReceivedPerInterface[i])
+ (networkIF.getBytesRecv() - bytesReceivedPerInterface.get(i))
* 1000
/ probeIntervalMs);
sendRatePerInterface.set(
i,
(networkIF.getBytesSent() - bytesSentPerInterface[i]) * 1000 / probeIntervalMs);
- bytesReceivedPerInterface[i] = networkIF.getBytesRecv();
+ bytesReceivedPerInterface.set(i, networkIF.getBytesRecv());
Review Comment:
if we want to be Thread safe should we not use [compareAndSet](https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/atomic/AtomicLongArray.html#compareAndSet-int-long-long-) rather than set?
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [hotfix] change bytesReceivedPerInterface to AtomicLongArray for thread safety [flink]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #26332:
URL: https://github.com/apache/flink/pull/26332#issuecomment-4358280073
This PR is being marked as stale since it has not had any activity in the last 90 days.
If you would like to keep this PR alive, please leave a comment asking for a review.
If the PR has merge conflicts, update it with the latest from the base branch.
If you are having difficulty finding a reviewer, please reach out to the
community, contact details can be found here: https://flink.apache.org/what-is-flink/community/
If this PR is no longer valid or desired, please feel free to close it.
If no activity occurs in the next 30 days, it will be automatically closed.
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [hotfix] change bytesReceivedPerInterface to AtomicLongArray for thread safety [flink]
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #26332:
URL: https://github.com/apache/flink/pull/26332#issuecomment-2741863351
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a1c34ee896281b7205f0dff341f5ad0e2d83a83a",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a1c34ee896281b7205f0dff341f5ad0e2d83a83a",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a1c34ee896281b7205f0dff341f5ad0e2d83a83a UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [hotfix] change bytesReceivedPerInterface to AtomicLongArray for thread safety [flink]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #26332:
URL: https://github.com/apache/flink/pull/26332#issuecomment-2989932628
This PR is being marked as stale since it has not had any activity in the last 90 days.
If you would like to keep this PR alive, please leave a comment asking for a review.
If the PR has merge conflicts, update it with the latest from the base branch.
If you are having difficulty finding a reviewer, please reach out to the
community, contact details can be found here: https://flink.apache.org/what-is-flink/community/
If this PR is no longer valid or desired, please feel free to close it.
If no activity occurs in the next 30 days, it will be automatically closed.
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [hotfix] change bytesReceivedPerInterface to AtomicLongArray for thread safety [flink]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on PR #26332:
URL: https://github.com/apache/flink/pull/26332#issuecomment-2743098691
I see other long arrays - why did you choose this one to change to be atomic? Can an instance of this class be used by more than one thread?
As this class extends thread - I was assuming that the instance of this class is only associated with one thread. I could imagine the system resources we are accessing like networkIF and cpu clicks could change underneath us if another thread updates them. But why does this variable need to be made atomic?
One way to show this needs changing would be to add a unit test that shows the how the existing long[] is problematic due to thread access.
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [hotfix] change bytesReceivedPerInterface to AtomicLongArray for thread safety [flink]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #26332:
URL: https://github.com/apache/flink/pull/26332#issuecomment-4586068458
This PR has been closed since it has not had any activity in 120 days.
If you feel like this was a mistake, or you would like to continue working on it,
please feel free to re-open the PR and ask for a review.
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [hotfix] change bytesReceivedPerInterface to AtomicLongArray for thread safety [flink]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #26332: [hotfix] change bytesReceivedPerInterface to AtomicLongArray for thread safety
URL: https://github.com/apache/flink/pull/26332
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org