[
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-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
