[jira] [Commented] (YARN-9269) Minor cleanup in FpgaResourceAllocator
[
https://issues.apache.org/jira/browse/YARN-9269?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16803117#comment-16803117
]
Hudson commented on YARN-9269:
--
SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #16295 (See
[https://builds.apache.org/job/Hadoop-trunk-Commit/16295/])
YARN-9269. Minor cleanup in FpgaResourceAllocator. Contributed by Peter
(devaraj: rev a4cd75e09c934699ec5e2fa969f1c8d0a14c1d49)
* (edit)
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/fpga/FpgaResourceAllocator.java
* (edit)
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/fpga/FpgaResourceHandlerImpl.java
> Minor cleanup in FpgaResourceAllocator
> --
>
> Key: YARN-9269
> URL: https://issues.apache.org/jira/browse/YARN-9269
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: Peter Bacsko
>Assignee: Peter Bacsko
>Priority: Minor
> Fix For: 3.3.0
>
> Attachments: YARN-9269-001.patch, YARN-9269-002.patch,
> YARN-9269-003.patch, YARN-9269-004.patch, YARN-9269-005.patch
>
>
> Some stuff that we observed:
> * {{addFpga()}} - we check for duplicate devices, but we don't print any
> error/warning if there's any.
> * {{findMatchedFpga()}} should be called {{findMatchingFpga()}}. Also, is
> this method even needed? We already receive an {{FpgaDevice}} instance in
> {{updateFpga()}} which I believe is the same that we're looking up.
> * variable {{IPIDpreference}} is confusing
> * {{availableFpga}} / {{usedFpgaByRequestor}} are instances of
> {{LinkedHashMap}}. What's the rationale behind this? Doesn't a simple
> {{HashMap}} suffice?
> * {{usedFpgaByRequestor}} should be renamed, naming is a bit unclear
> * {{allowedFpgas}} should be an immutable list
> * {{@VisibleForTesting}} methods should be package private
> * get rid of {{*}} imports
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-9269) Minor cleanup in FpgaResourceAllocator
[
https://issues.apache.org/jira/browse/YARN-9269?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16802618#comment-16802618
]
Peter Bacsko commented on YARN-9269:
[~devaraj.k] [~sunilg] Could you guys review + commit it if it looks good?
Thanks!
> Minor cleanup in FpgaResourceAllocator
> --
>
> Key: YARN-9269
> URL: https://issues.apache.org/jira/browse/YARN-9269
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: Peter Bacsko
>Assignee: Peter Bacsko
>Priority: Major
> Attachments: YARN-9269-001.patch, YARN-9269-002.patch,
> YARN-9269-003.patch, YARN-9269-004.patch, YARN-9269-005.patch
>
>
> Some stuff that we observed:
> * {{addFpga()}} - we check for duplicate devices, but we don't print any
> error/warning if there's any.
> * {{findMatchedFpga()}} should be called {{findMatchingFpga()}}. Also, is
> this method even needed? We already receive an {{FpgaDevice}} instance in
> {{updateFpga()}} which I believe is the same that we're looking up.
> * variable {{IPIDpreference}} is confusing
> * {{availableFpga}} / {{usedFpgaByRequestor}} are instances of
> {{LinkedHashMap}}. What's the rationale behind this? Doesn't a simple
> {{HashMap}} suffice?
> * {{usedFpgaByRequestor}} should be renamed, naming is a bit unclear
> * {{allowedFpgas}} should be an immutable list
> * {{@VisibleForTesting}} methods should be package private
> * get rid of {{*}} imports
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-9269) Minor cleanup in FpgaResourceAllocator
[
https://issues.apache.org/jira/browse/YARN-9269?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16801158#comment-16801158
]
Hadoop QA commented on YARN-9269:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
13s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 16m
33s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
2s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
25s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
40s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
11m 20s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
56s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
23s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
34s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
58s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
58s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
22s{color} | {color:green}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:
The patch generated 0 new + 27 unchanged - 3 fixed = 27 total (was 30) {color}
|
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
35s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
11m 30s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
1s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
23s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 21m
15s{color} | {color:green} hadoop-yarn-server-nodemanager in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
29s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 68m 34s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8f97d6f |
| JIRA Issue | YARN-9269 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12963657/YARN-9269-005.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 2660c18d6cae 4.4.0-139-generic #165-Ubuntu SMP Wed Oct 24
10:58:50 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / 710cbc9 |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_191 |
| findbugs | v3.1.0-RC1 |
| Test Results |
https://builds.apache.org/job/PreCommit-YARN-Build/23806/testReport/ |
| Max. process+thread count | 447 (vs. ulimit of 1) |
| modules | C:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
U:
[jira] [Commented] (YARN-9269) Minor cleanup in FpgaResourceAllocator
[
https://issues.apache.org/jira/browse/YARN-9269?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16801043#comment-16801043
]
Peter Bacsko commented on YARN-9269:
Uploaded patch v5, because v4 no longer applies to trunk cleanly.
> Minor cleanup in FpgaResourceAllocator
> --
>
> Key: YARN-9269
> URL: https://issues.apache.org/jira/browse/YARN-9269
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: Peter Bacsko
>Assignee: Peter Bacsko
>Priority: Major
> Attachments: YARN-9269-001.patch, YARN-9269-002.patch,
> YARN-9269-003.patch, YARN-9269-004.patch, YARN-9269-005.patch
>
>
> Some stuff that we observed:
> * {{addFpga()}} - we check for duplicate devices, but we don't print any
> error/warning if there's any.
> * {{findMatchedFpga()}} should be called {{findMatchingFpga()}}. Also, is
> this method even needed? We already receive an {{FpgaDevice}} instance in
> {{updateFpga()}} which I believe is the same that we're looking up.
> * variable {{IPIDpreference}} is confusing
> * {{availableFpga}} / {{usedFpgaByRequestor}} are instances of
> {{LinkedHashMap}}. What's the rationale behind this? Doesn't a simple
> {{HashMap}} suffice?
> * {{usedFpgaByRequestor}} should be renamed, naming is a bit unclear
> * {{allowedFpgas}} should be an immutable list
> * {{@VisibleForTesting}} methods should be package private
> * get rid of {{*}} imports
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-9269) Minor cleanup in FpgaResourceAllocator
[
https://issues.apache.org/jira/browse/YARN-9269?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16800484#comment-16800484
]
Peter Bacsko commented on YARN-9269:
[~devaraj.k] could you review this as well?
> Minor cleanup in FpgaResourceAllocator
> --
>
> Key: YARN-9269
> URL: https://issues.apache.org/jira/browse/YARN-9269
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: Peter Bacsko
>Assignee: Peter Bacsko
>Priority: Major
> Attachments: YARN-9269-001.patch, YARN-9269-002.patch,
> YARN-9269-003.patch, YARN-9269-004.patch
>
>
> Some stuff that we observed:
> * {{addFpga()}} - we check for duplicate devices, but we don't print any
> error/warning if there's any.
> * {{findMatchedFpga()}} should be called {{findMatchingFpga()}}. Also, is
> this method even needed? We already receive an {{FpgaDevice}} instance in
> {{updateFpga()}} which I believe is the same that we're looking up.
> * variable {{IPIDpreference}} is confusing
> * {{availableFpga}} / {{usedFpgaByRequestor}} are instances of
> {{LinkedHashMap}}. What's the rationale behind this? Doesn't a simple
> {{HashMap}} suffice?
> * {{usedFpgaByRequestor}} should be renamed, naming is a bit unclear
> * {{allowedFpgas}} should be an immutable list
> * {{@VisibleForTesting}} methods should be package private
> * get rid of {{*}} imports
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-9269) Minor cleanup in FpgaResourceAllocator
[
https://issues.apache.org/jira/browse/YARN-9269?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16800180#comment-16800180
]
Hadoop QA commented on YARN-9269:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
14s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 16m
29s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
3s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
29s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
41s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
12m 9s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
56s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
27s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
34s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
58s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
58s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
23s{color} | {color:green}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:
The patch generated 0 new + 34 unchanged - 3 fixed = 34 total (was 37) {color}
|
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
35s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
11m 59s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
5s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
25s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 21m
22s{color} | {color:green} hadoop-yarn-server-nodemanager in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
28s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 70m 25s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8f97d6f |
| JIRA Issue | YARN-9269 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12963561/YARN-9269-004.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 2b25bca93746 4.4.0-139-generic #165-Ubuntu SMP Wed Oct 24
10:58:50 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / 128dd91 |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_191 |
| findbugs | v3.1.0-RC1 |
| Test Results |
https://builds.apache.org/job/PreCommit-YARN-Build/23796/testReport/ |
| Max. process+thread count | 412 (vs. ulimit of 1) |
| modules | C:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
U:
[jira] [Commented] (YARN-9269) Minor cleanup in FpgaResourceAllocator
[
https://issues.apache.org/jira/browse/YARN-9269?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16795674#comment-16795674
]
Devaraj K commented on YARN-9269:
-
Thanks [~pbacsko] for the patch, latest patch is not getting applied, please
update it.
> Minor cleanup in FpgaResourceAllocator
> --
>
> Key: YARN-9269
> URL: https://issues.apache.org/jira/browse/YARN-9269
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: Peter Bacsko
>Assignee: Peter Bacsko
>Priority: Major
> Attachments: YARN-9269-001.patch, YARN-9269-002.patch,
> YARN-9269-003.patch
>
>
> Some stuff that we observed:
> * {{addFpga()}} - we check for duplicate devices, but we don't print any
> error/warning if there's any.
> * {{findMatchedFpga()}} should be called {{findMatchingFpga()}}. Also, is
> this method even needed? We already receive an {{FpgaDevice}} instance in
> {{updateFpga()}} which I believe is the same that we're looking up.
> * variable {{IPIDpreference}} is confusing
> * {{availableFpga}} / {{usedFpgaByRequestor}} are instances of
> {{LinkedHashMap}}. What's the rationale behind this? Doesn't a simple
> {{HashMap}} suffice?
> * {{usedFpgaByRequestor}} should be renamed, naming is a bit unclear
> * {{allowedFpgas}} should be an immutable list
> * {{@VisibleForTesting}} methods should be package private
> * get rid of {{*}} imports
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-9269) Minor cleanup in FpgaResourceAllocator
[
https://issues.apache.org/jira/browse/YARN-9269?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16768154#comment-16768154
]
Hadoop QA commented on YARN-9269:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
23s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 18m
17s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
30s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
33s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
47s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
13m 45s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
14s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
33s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
42s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
18s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 1m
18s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
29s{color} | {color:green}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:
The patch generated 0 new + 50 unchanged - 8 fixed = 50 total (was 58) {color}
|
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
44s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
14m 21s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
18s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
29s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 22m
13s{color} | {color:green} hadoop-yarn-server-nodemanager in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
28s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 79m 9s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8f97d6f |
| JIRA Issue | YARN-9269 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12958705/YARN-9269-003.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 0c5593d0a8da 4.4.0-138-generic #164~14.04.1-Ubuntu SMP Fri Oct
5 08:56:16 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / dfe0f42 |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_191 |
| findbugs | v3.1.0-RC1 |
| Test Results |
https://builds.apache.org/job/PreCommit-YARN-Build/23402/testReport/ |
| Max. process+thread count | 305 (vs. ulimit of 1) |
| modules | C:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
U:
[jira] [Commented] (YARN-9269) Minor cleanup in FpgaResourceAllocator
[
https://issues.apache.org/jira/browse/YARN-9269?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16768129#comment-16768129
]
Adam Antal commented on YARN-9269:
--
Perfect, +1 (non-binding).
> Minor cleanup in FpgaResourceAllocator
> --
>
> Key: YARN-9269
> URL: https://issues.apache.org/jira/browse/YARN-9269
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: Peter Bacsko
>Assignee: Peter Bacsko
>Priority: Major
> Attachments: YARN-9269-001.patch, YARN-9269-002.patch,
> YARN-9269-003.patch
>
>
> Some stuff that we observed:
> * {{addFpga()}} - we check for duplicate devices, but we don't print any
> error/warning if there's any.
> * {{findMatchedFpga()}} should be called {{findMatchingFpga()}}. Also, is
> this method even needed? We already receive an {{FpgaDevice}} instance in
> {{updateFpga()}} which I believe is the same that we're looking up.
> * variable {{IPIDpreference}} is confusing
> * {{availableFpga}} / {{usedFpgaByRequestor}} are instances of
> {{LinkedHashMap}}. What's the rationale behind this? Doesn't a simple
> {{HashMap}} suffice?
> * {{usedFpgaByRequestor}} should be renamed, naming is a bit unclear
> * {{allowedFpgas}} should be an immutable list
> * {{@VisibleForTesting}} methods should be package private
> * get rid of {{*}} imports
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-9269) Minor cleanup in FpgaResourceAllocator
[
https://issues.apache.org/jira/browse/YARN-9269?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16762983#comment-16762983
]
Adam Antal commented on YARN-9269:
--
Thanks you for the answers, I agree. Sorry for missing YARN-9268, I'll take a
look at that as well soon.
Maybe allowedFpgas could be renamed to reflect that it is a kind of initializer
(so it is called only once), but it's ok how this is now.
Pending new patch and jenkins, but the patch looks good to me.
> Minor cleanup in FpgaResourceAllocator
> --
>
> Key: YARN-9269
> URL: https://issues.apache.org/jira/browse/YARN-9269
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: Peter Bacsko
>Assignee: Peter Bacsko
>Priority: Major
> Attachments: YARN-9269-001.patch, YARN-9269-002.patch
>
>
> Some stuff that we observed:
> * {{addFpga()}} - we check for duplicate devices, but we don't print any
> error/warning if there's any.
> * {{findMatchedFpga()}} should be called {{findMatchingFpga()}}. Also, is
> this method even needed? We already receive an {{FpgaDevice}} instance in
> {{updateFpga()}} which I believe is the same that we're looking up.
> * variable {{IPIDpreference}} is confusing
> * {{availableFpga}} / {{usedFpgaByRequestor}} are instances of
> {{LinkedHashMap}}. What's the rationale behind this? Doesn't a simple
> {{HashMap}} suffice?
> * {{usedFpgaByRequestor}} should be renamed, naming is a bit unclear
> * {{allowedFpgas}} should be an immutable list
> * {{@VisibleForTesting}} methods should be package private
> * get rid of {{*}} imports
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-9269) Minor cleanup in FpgaResourceAllocator
[
https://issues.apache.org/jira/browse/YARN-9269?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16762946#comment-16762946
]
Peter Bacsko commented on YARN-9269:
Thanks [~adam.antal] for the comments.
1. {{allowedFpgas}} is populated only once and never modified again. It's
called from the {{bootstrap()}} method when the NM starts up. So I add the
elements to a local list then wrap it in an immutable list.
2. Yeah but that technically cannot happen. Also, we no longer retrieve the
device object from a list - we have it already as a method argument. Anyway
I'll have to double check that this is still OK (I haven't tested any
modifications on a cluster yet).
3. Unused stuff in FpgaDevice: yeah, there's a separate Jira for that:
YARN-9268. We store stuff which is unnecessary. Only aliasDevName, minor and
major are important.
4. Ok, I will do the rename + remove the extra spaces.
> Minor cleanup in FpgaResourceAllocator
> --
>
> Key: YARN-9269
> URL: https://issues.apache.org/jira/browse/YARN-9269
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: Peter Bacsko
>Assignee: Peter Bacsko
>Priority: Major
> Attachments: YARN-9269-001.patch, YARN-9269-002.patch
>
>
> Some stuff that we observed:
> * {{addFpga()}} - we check for duplicate devices, but we don't print any
> error/warning if there's any.
> * {{findMatchedFpga()}} should be called {{findMatchingFpga()}}. Also, is
> this method even needed? We already receive an {{FpgaDevice}} instance in
> {{updateFpga()}} which I believe is the same that we're looking up.
> * variable {{IPIDpreference}} is confusing
> * {{availableFpga}} / {{usedFpgaByRequestor}} are instances of
> {{LinkedHashMap}}. What's the rationale behind this? Doesn't a simple
> {{HashMap}} suffice?
> * {{usedFpgaByRequestor}} should be renamed, naming is a bit unclear
> * {{allowedFpgas}} should be an immutable list
> * {{@VisibleForTesting}} methods should be package private
> * get rid of {{*}} imports
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-9269) Minor cleanup in FpgaResourceAllocator
[
https://issues.apache.org/jira/browse/YARN-9269?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16762899#comment-16762899
]
Adam Antal commented on YARN-9269:
--
Thanks for the patch, [~pbacsko]. Nice job, the quality of the code is much
better now. Here are some thoughts of mine:
- Could you please rationale why has the behaviour of addFpga changed?
Previously the devices from the variable list (also there can be some more
descriptive name of it) is processed this way: devices are being added one by
one to allowedFpgas list, if they're not already added. Now we collect them
into a list named fpgaDevices, and at the end of the function we overwrite the
list allowedFpgas, so the devices added to the allowedFpgas list before the
function is called can be lost.
- I agree that the function findMatchedFpga is not needed (basically just a
type of search/"indexOf" function), but the updateFpga function's side effect
is that it also checks whether usedFpgaByRequestor (renamed to
containerToFpgaMapping) contains that device or not, and if not contained a
warning is raised. Even though this function is only called from
FpgaResourceHandlerImpl:preStart, where we know that this case does not happen,
I think we should explicitly keep that.
- There are some unused function in FpgaDevice class: setDevName,
setAliasDevName, setBusNum. It makes sense to have getters and setters for all
the class variables, but they're also missing from temperature, and card power
usage, so I think the above mentioned function could be removed.
- It's really a minor thing, but can we rename the class variable availableFpga
to availableFpgas (so an extra s, implying its more than one device).
- There are also useless extra spaces on the top of the file, right after the
license and after the package statement.
Please enlight me, if I don't see something clear.
> Minor cleanup in FpgaResourceAllocator
> --
>
> Key: YARN-9269
> URL: https://issues.apache.org/jira/browse/YARN-9269
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: Peter Bacsko
>Assignee: Peter Bacsko
>Priority: Major
> Attachments: YARN-9269-001.patch, YARN-9269-002.patch
>
>
> Some stuff that we observed:
> * {{addFpga()}} - we check for duplicate devices, but we don't print any
> error/warning if there's any.
> * {{findMatchedFpga()}} should be called {{findMatchingFpga()}}. Also, is
> this method even needed? We already receive an {{FpgaDevice}} instance in
> {{updateFpga()}} which I believe is the same that we're looking up.
> * variable {{IPIDpreference}} is confusing
> * {{availableFpga}} / {{usedFpgaByRequestor}} are instances of
> {{LinkedHashMap}}. What's the rationale behind this? Doesn't a simple
> {{HashMap}} suffice?
> * {{usedFpgaByRequestor}} should be renamed, naming is a bit unclear
> * {{allowedFpgas}} should be an immutable list
> * {{@VisibleForTesting}} methods should be package private
> * get rid of {{*}} imports
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-9269) Minor cleanup in FpgaResourceAllocator
[
https://issues.apache.org/jira/browse/YARN-9269?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16761762#comment-16761762
]
Hadoop QA commented on YARN-9269:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
31s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 24m
0s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
22s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
33s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
45s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
13m 48s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
18s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
29s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
43s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
13s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 1m
13s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
26s{color} | {color:green}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:
The patch generated 0 new + 21 unchanged - 8 fixed = 21 total (was 29) {color}
|
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
44s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
14m 18s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
30s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
30s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 22m
7s{color} | {color:green} hadoop-yarn-server-nodemanager in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
53s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 85m 21s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8f97d6f |
| JIRA Issue | YARN-9269 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12957753/YARN-9269-002.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 173a6c9a8cae 4.4.0-138-generic #164~14.04.1-Ubuntu SMP Fri Oct
5 08:56:16 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / 911790c |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_191 |
| findbugs | v3.1.0-RC1 |
| Test Results |
https://builds.apache.org/job/PreCommit-YARN-Build/23313/testReport/ |
| Max. process+thread count | 338 (vs. ulimit of 1) |
| modules | C:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
U:
[jira] [Commented] (YARN-9269) Minor cleanup in FpgaResourceAllocator
[
https://issues.apache.org/jira/browse/YARN-9269?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16761695#comment-16761695
]
Hadoop QA commented on YARN-9269:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
13s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
1s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 23m
39s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
5s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
29s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
43s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
12m 18s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
3s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
27s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
36s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
0s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 1m
0s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 22s{color} | {color:orange}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:
The patch generated 4 new + 22 unchanged - 7 fixed = 26 total (was 29) {color}
|
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
37s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
12m 30s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
8s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
22s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 21m
12s{color} | {color:green} hadoop-yarn-server-nodemanager in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
28s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 78m 3s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8f97d6f |
| JIRA Issue | YARN-9269 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12957743/YARN-9269-001.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux f3d9f1ce031c 4.4.0-139-generic #165~14.04.1-Ubuntu SMP Wed Oct
31 10:55:11 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / 911790c |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_191 |
| findbugs | v3.1.0-RC1 |
| checkstyle |
https://builds.apache.org/job/PreCommit-YARN-Build/23312/artifact/out/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
|
| Test Results |
https://builds.apache.org/job/PreCommit-YARN-Build/23312/testReport/ |
| Max. process+thread count | 308
