You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "cxzl25 (via GitHub)" <gi...@apache.org> on 2023/12/21 05:24:44 UTC
[PR] [CELEBORN-1190][FOLLOWUP] Apply error prone patch and suppress some problems [incubator-celeborn]
cxzl25 opened a new pull request, #2180:
URL: https://github.com/apache/incubator-celeborn/pull/2180
### What changes were proposed in this pull request?
1. Fix IntLongMath, InconsistentCapitalization, UnnecessaryAssignment
2. disable StringSplitter, EmptyBlockTag, EqualsGetClass, MissingSummary, BadImport
### Why are the changes needed?
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
GA
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1190][FOLLOWUP] Apply error prone patch and suppress some problems [incubator-celeborn]
Posted by "waitinfuture (via GitHub)" <gi...@apache.org>.
waitinfuture commented on PR #2180:
URL: https://github.com/apache/incubator-celeborn/pull/2180#issuecomment-1874043582
> IMO, it would be better to hold this PR until there is a solution to add the error-prone plugin into SBT.
Hi @FMX , I think we don't require this PR to also support SBT before merging. It's totally OK to do it in future.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1190][FOLLOWUP] Apply error prone patch and suppress some problems [incubator-celeborn]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #2180:
URL: https://github.com/apache/incubator-celeborn/pull/2180#discussion_r1433630686
##########
worker/src/main/java/org/apache/celeborn/service/deploy/worker/storage/CreditStreamManager.java:
##########
@@ -55,21 +55,21 @@ public class CreditStreamManager {
public CreditStreamManager(
int minReadBuffers,
int maxReadBuffers,
- int threadsPerMountpoint,
+ int threadsPerMountPoint,
int minBuffersToTriggerRead) {
nextStreamId = new AtomicLong((long) new Random().nextInt(Integer.MAX_VALUE) * 1000);
streams = JavaUtils.newConcurrentHashMap();
activeMapPartitions = JavaUtils.newConcurrentHashMap();
this.minReadBuffers = minReadBuffers;
this.maxReadBuffers = maxReadBuffers;
- threadsPerMountPoint = threadsPerMountpoint;
Review Comment:
We just use different variable names here to avoid this problem.
https://errorprone.info/bugpattern/InconsistentCapitalization
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1190][FOLLOWUP] Apply error prone patch and suppress some problems [incubator-celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #2180:
URL: https://github.com/apache/incubator-celeborn/pull/2180#discussion_r1433627756
##########
worker/src/main/java/org/apache/celeborn/service/deploy/worker/storage/CreditStreamManager.java:
##########
@@ -55,21 +55,21 @@ public class CreditStreamManager {
public CreditStreamManager(
int minReadBuffers,
int maxReadBuffers,
- int threadsPerMountpoint,
+ int threadsPerMountPoint,
int minBuffersToTriggerRead) {
nextStreamId = new AtomicLong((long) new Random().nextInt(Integer.MAX_VALUE) * 1000);
streams = JavaUtils.newConcurrentHashMap();
activeMapPartitions = JavaUtils.newConcurrentHashMap();
this.minReadBuffers = minReadBuffers;
this.maxReadBuffers = maxReadBuffers;
- threadsPerMountPoint = threadsPerMountpoint;
Review Comment:
so this is a bug?
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1190][FOLLOWUP] Apply error prone patch and suppress some problems [incubator-celeborn]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #2180:
URL: https://github.com/apache/incubator-celeborn/pull/2180#issuecomment-1865510468
## [Codecov](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2180?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: `1 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`8a34c37`)](https://app.codecov.io/gh/apache/incubator-celeborn/commit/8a34c376cb4cbaaf0679f2c59288015c70107a73?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 47.10% compared to head [(`7335661`)](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2180?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 47.09%.
| [Files](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2180?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...orn/common/network/util/TransportFrameDecoder.java](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2180?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jZWxlYm9ybi9jb21tb24vbmV0d29yay91dGlsL1RyYW5zcG9ydEZyYW1lRGVjb2Rlci5qYXZh) | 50.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2180?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #2180 +/- ##
==========================================
- Coverage 47.10% 47.09% -0.00%
==========================================
Files 185 185
Lines 11675 11675
Branches 1041 1041
==========================================
- Hits 5498 5497 -1
Misses 5809 5809
- Partials 368 369 +1
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2180?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1190][FOLLOWUP] Apply error prone patch and suppress some problems [incubator-celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #2180:
URL: https://github.com/apache/incubator-celeborn/pull/2180#discussion_r1433663988
##########
worker/src/main/java/org/apache/celeborn/service/deploy/worker/storage/CreditStreamManager.java:
##########
@@ -55,21 +55,21 @@ public class CreditStreamManager {
public CreditStreamManager(
int minReadBuffers,
int maxReadBuffers,
- int threadsPerMountpoint,
+ int threadsPerMountPoint,
int minBuffersToTriggerRead) {
nextStreamId = new AtomicLong((long) new Random().nextInt(Integer.MAX_VALUE) * 1000);
streams = JavaUtils.newConcurrentHashMap();
activeMapPartitions = JavaUtils.newConcurrentHashMap();
this.minReadBuffers = minReadBuffers;
this.maxReadBuffers = maxReadBuffers;
- threadsPerMountPoint = threadsPerMountpoint;
Review Comment:
Oh, I see
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1190][FOLLOWUP] Apply error prone patch and suppress some problems [incubator-celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #2180:
URL: https://github.com/apache/incubator-celeborn/pull/2180#discussion_r1433628164
##########
worker/src/main/java/org/apache/celeborn/service/deploy/worker/storage/CreditStreamManager.java:
##########
@@ -55,21 +55,21 @@ public class CreditStreamManager {
public CreditStreamManager(
int minReadBuffers,
int maxReadBuffers,
- int threadsPerMountpoint,
+ int threadsPerMountPoint,
int minBuffersToTriggerRead) {
nextStreamId = new AtomicLong((long) new Random().nextInt(Integer.MAX_VALUE) * 1000);
streams = JavaUtils.newConcurrentHashMap();
activeMapPartitions = JavaUtils.newConcurrentHashMap();
this.minReadBuffers = minReadBuffers;
this.maxReadBuffers = maxReadBuffers;
- threadsPerMountPoint = threadsPerMountpoint;
Review Comment:
does it cause a real impact?
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1190][FOLLOWUP] Apply error prone patch and suppress some problems [incubator-celeborn]
Posted by "waitinfuture (via GitHub)" <gi...@apache.org>.
waitinfuture closed pull request #2180: [CELEBORN-1190][FOLLOWUP] Apply error prone patch and suppress some problems
URL: https://github.com/apache/incubator-celeborn/pull/2180
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1190][FOLLOWUP] Apply error prone patch and suppress some problems [incubator-celeborn]
Posted by "FMX (via GitHub)" <gi...@apache.org>.
FMX commented on PR #2180:
URL: https://github.com/apache/incubator-celeborn/pull/2180#issuecomment-1869919412
I have a question that the maven build system will be dropped after v0.5.0 or later. Is this necessary? Will error prone be integrated into SBT?
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1190][FOLLOWUP] Apply error prone patch and suppress some problems [incubator-celeborn]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on PR #2180:
URL: https://github.com/apache/incubator-celeborn/pull/2180#issuecomment-1869945286
> Will error prone be integrated into SBT
A JIRA ticket has been created to track this issue. [CELEBORN-1080](https://issues.apache.org/jira/browse/CELEBORN-1080)
Tried it locally but not successful yet.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1190][FOLLOWUP] Apply error prone patch and suppress some problems [incubator-celeborn]
Posted by "FMX (via GitHub)" <gi...@apache.org>.
FMX commented on PR #2180:
URL: https://github.com/apache/incubator-celeborn/pull/2180#issuecomment-1870736577
IMO, it would be better to hold this PR until there is a solution to add the error-prone plugin into SBT.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org