[
https://issues.apache.org/jira/browse/HDFS-7723?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14314682#comment-14314682
]
Xiaoyu Yao commented on HDFS-7723:
----------------------------------
Thanks [~arpitagarwal] again for reviewing the patch.
bq. 1. More of a question: In many places where setQuota takes the (nsQuota,
dsQuota, type) triplet as parameter, nsQuota should be QUOTA_DONT_SET if type
is non-null. e.g. in unprotectedSetQuota you throw an exception if types is
non-NULL and nsQuota is non-zero. However methods where you take (nsQuota,
dsQuota and EnumSet) can have any combination. Is this correct?
Do you mean the DirectoryWithQuotaFeature#setSpaceConsumed() which takes
(nsQutoa,dsQuota and EnumSet)? This is an internal method only used for
FsImage loading where the namespace quota, storage space quota and storage type
quota of a directory can be deserialized together to update the in-memory quota
counter in a single call.
>From API level, we only take the (nsQuota, dsQuota, type) triplet when quota
>is set. Because the setQuota RPC protocol message is reused, dsQuota is shared
>between storage space quota and the storage type quota depending on whether
>type is null or not. We want to keep the existing behavior where user can set
>the name space quota and storage space quota together. But when setting the
>storage type quota, we don't allow setting namespace quota together with
>storage type quota for clarity as name space quota does not apply to storage
>type. We could change this if it is preferrable to allow setting both name
>space quota and storage type quota.
bq. 2. Error message in FsImage#updateCountForQuota is wrong. A quota violation
in the image is not necessarily a bug. The existing "BUG" error messages are
also wrong. I filed HDFS-7757 to fix them. Also this loop may be a performance
issue when loading millions of INodes. Since all it does is check violation and
log a message, we should just remove it.
Agree, we should at least change it to a warning or just initialize the
in-memory quota counters without enforcing check during fsimage load. Let's fix
it in HDFS-7757.
bq. 3. FSImageFormat.java:915 - did you miss passing typeCounts to
copyWithQuota constructor?
This is based on that we don't need to handle quota by storage type for legacy
fsimages, which is handled in this FSImageFormat class (used by legacy_oiv
only). The current fsimage processing is done in the protobuf based class
FSImageFormatProtobuf, which is updated with typeCounts in the patch.
bq. 4. QuotaCounts: typeSpaces and typeCounts are used interchangeably. We
should probably name them consistently.
Agree that we should name them consistently. Is it OK that we do it in a
separate JIRA for refactoring names together with issue 6 metioned below as the
current patch is already pretty large already?
bq. 5. Nitpick: Rename anyGreatOrEqual to anyGreaterOrEqual
Agree and will fix in the next update.
bq. 6. You renamed DEFAULT_DISKSPACE_QUOTA to DEFAULT_SPACE_QUOTA.
DEFAULT_STORAGE_SPACE_QUOTA would be easier to understand. Comments like
@return the namespace and diskspace consumed can also be edited to use storage
space instead of diskspace. We can probably do this in a separate patch too,
the current patch is large enough already!
Agree and let's fix it in a follow up refactor JIRA.
> Quota By Storage Type namenode implemenation
> --------------------------------------------
>
> Key: HDFS-7723
> URL: https://issues.apache.org/jira/browse/HDFS-7723
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: datanode, namenode
> Reporter: Xiaoyu Yao
> Assignee: Xiaoyu Yao
> Attachments: HDFS-7723.0.patch, HDFS-7723.1.patch, HDFS-7723.2.patch,
> HDFS-7723.3.patch, HDFS-7723.4.patch
>
>
> This includes: 1) new editlog to persist quota by storage type op 2)
> corresponding fsimage load/save the new op. 3) QuotaCount refactor to update
> usage of the storage types for quota enforcement 4) Snapshot support 5) Unit
> test update
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
