hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Adam Antal (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (YARN-9269) Minor cleanup in FpgaResourceAllocator
Date Thu, 07 Feb 2019 19:11:00 GMT

    [ https://issues.apache.org/jira/browse/YARN-9269?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16762983#comment-16762983
] 

Adam Antal edited comment on YARN-9269 at 2/7/19 7:10 PM:
----------------------------------------------------------

Thanks you for the answers, I agree. Sorry for missing YARN-9268, I'll take a look at that
as well soon.
Maybe addFpgas could be renamed (like populate or something) to reflect that it is a kind
of initializer (so it is called only once), but it's ok how this is now, and maybe we shouldn't
bother other files, as it would be other than the purpose of this jira.
Pending new patch and jenkins, but the patch looks good to me. 



was (Author: adam.antal):
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-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org


Mime
View raw message