Skip to content

HDFS-15624. fix the function of setting quota by storage type #2377

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
liuml07 merged 20 commits into from
Feb 3, 2021
Merged

HDFS-15624. fix the function of setting quota by storage type #2377

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
7736e7d
fix the function of setting quota by storage type
YaYun-Wang Oct 10, 2020
3e62104
fix the function of setting quota by storage type
YaYun-Wang Oct 12, 2020
0a731cd
fix the function of setting quota by storage type
YaYun-Wang Oct 12, 2020
fcc143d
handle Namenode's layout version while upfrade or downgrade
YaYun-Wang Oct 13, 2020
113f0de
handle Namenode's layout version while upfrade or downgrade
YaYun-Wang Oct 13, 2020
2d3bb85
modify the ancestorLV of new features
YaYun-Wang Oct 15, 2020
edfeec0
just fix the bug of SetQuotaByStorageTypeOp
YaYun-Wang Oct 21, 2020
d7db1b9
just fix the bug of SetQuotaByStorageTypeOp
YaYun-Wang Oct 22, 2020
c65741a
just fix the bug of SetQuotaByStorageTypeOp
YaYun-Wang Oct 22, 2020
5a99d1b
just fix the bug of SetQuotaByStorageTypeOp
YaYun-Wang Oct 22, 2020
175f33e
test
YaYun-Wang Oct 22, 2020
2ca2407
add new feature again
YaYun-Wang Oct 22, 2020
847b8f7
fix BLOCK_STORAGE_POLICY errors of support in FSEditLogOp
YaYun-Wang Oct 22, 2020
769a7c1
delete extra whitespace
YaYun-Wang Oct 23, 2020
92af855
Adjust the Namenode's layout version and add new function judgment to…
kevinbrandon2020 Oct 26, 2020
6ba9605
Adjust the Namenode's layout version and add new function judgment to…
kevinbrandon2020 Oct 27, 2020
e3eadaa
Adjust the Namenode's layout version and add new function judgment to…
kevinbrandon2020 Oct 27, 2020
a88eb6c
Adjust the Namenode's layout version and add new function judgment to…
kevinbrandon2020 Oct 27, 2020
56bcbbd
HDFS-15624 Fix NVDIMM storage issues
huangtianhua Oct 30, 2020
15f0386
Merge branch 'trunk' into HDFS-15624
huangtianhua Dec 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/StorageType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,12 @@
@InterfaceAudience.Public
@InterfaceStability.Unstable
public enum StorageType {
// sorted by the speed of the storage types, from fast to slow
RAM_DISK(true, true),
NVDIMM(false, true),
SSD(false, false),
DISK(false, false),
ARCHIVE(false, false),
PROVIDED(false, false);
PROVIDED(false, false),
NVDIMM(false, true);

Comment on lines 40 to 43
Copy link
Contributor

@vinayakumarb vinayakumarb Oct 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of addition of new STORAGE TYPE,
during upgrade
calls from old clients, who doesnt have this storagetype, will experience failure.

Especially getQuotaUsage() call will experience failure. May be need to analyze more to avoid failures during upgrade.
Even though upgrade is finalized, old clients should continue to work.

Copy link
Contributor

@huangtianhua huangtianhua Oct 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We add check for setQuota() in FSNamesystem.java, I think it's ok, right?

Copy link
Contributor

@vinayakumarb vinayakumarb Oct 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setQuota() check will only block during rollingupgrade. But once its finalized, still old clients experience failure.
Anyway thats the current problem, even before this feature. Can be handled in a separate Jira.

More details about failure: 2.10.1 client asking quota usage from 3.3.0 namenode.

$ bin/hdfs dfs -fs hdfs://namenode:8020/ -count -q -t -h /
count: Message missing required fields: usage.typeQuotaInfos.typeQuotaInfo[3].type

Above issue is coming, because 2.10.1 client doent know about PROVIDED StorageType.
Similar problem will occur for NVDIMM also from previous version clients.

Copy link
Member

@ayushtkn ayushtkn Oct 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure but will getStoragePolicies also land up in something similar issue? Due to unavailability of storage type? The quota stuff shall be there for PROVIDED also but in case this backward incompatibility is there with Storage Policy too, Then we need to find out some way.
@vinayakumarb do you have pointers or suggestions on this, how to tackle this?

Copy link
Contributor

@vinayakumarb vinayakumarb Oct 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have verified the getStoragePolicies() with older clients.
Older clients will get the storage policy but, unknown storage types will be ignored.
So in this case, ALLNVDIMM storage policy shows empty StorageTypes for older clients.

May be need to show DEFAULT StorageType instead of ignoring the unknown StorageTypes. This also can be fixed in a separate Jira. Right now, existing clients wont be broken on getStoragePolicies() call with this change.

So nothing special required for that in this PR.

Copy link
Contributor

@huangtianhua huangtianhua Oct 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarification and testing.

private final boolean isTransient;
private final boolean isRAM;
Expand Down Expand Up @@ -117,4 +116,4 @@ public static String getConf(Configuration conf,
StorageType t, String name) {
return conf.get(CONF_KEY_HEADER + t.toString() + "." + name);
}
}
}
4 changes: 2 additions & 2 deletions hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCount.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,10 +283,10 @@ public void processPathWithQuotasByStorageTypesHeader() throws Exception {
count.processOptions(options);
String withStorageTypeHeader =
// <----13---> <-------17------> <----13-----> <------17------->
" NVDIMM_QUOTA REM_NVDIMM_QUOTA " +
" SSD_QUOTA REM_SSD_QUOTA DISK_QUOTA REM_DISK_QUOTA " +
// <----13---> <-------17------>
"ARCHIVE_QUOTA REM_ARCHIVE_QUOTA PROVIDED_QUOTA REM_PROVIDED_QUOTA " +
" NVDIMM_QUOTA REM_NVDIMM_QUOTA " +
"PATHNAME";
verify(out).println(withStorageTypeHeader);
verifyNoMoreInteractions(out);
Expand Down Expand Up @@ -338,11 +338,11 @@ public void processPathWithQuotasByQTVH() throws Exception {
count.processOptions(options);
String withStorageTypeHeader =
// <----13---> <-------17------>
" NVDIMM_QUOTA REM_NVDIMM_QUOTA " +
" SSD_QUOTA REM_SSD_QUOTA " +
" DISK_QUOTA REM_DISK_QUOTA " +
"ARCHIVE_QUOTA REM_ARCHIVE_QUOTA " +
"PROVIDED_QUOTA REM_PROVIDED_QUOTA " +
" NVDIMM_QUOTA REM_NVDIMM_QUOTA " +
"PATHNAME";
verify(out).println(withStorageTypeHeader);
verifyNoMoreInteractions(out);
Expand Down
14 changes: 7 additions & 7 deletions ...fs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterQuota.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,13 +414,13 @@ public void testStorageTypeQuota() throws Exception {
QuotaUsage usage = client.getQuotaUsage("/type0");
assertEquals(HdfsConstants.QUOTA_RESET, usage.getQuota());
assertEquals(HdfsConstants.QUOTA_RESET, usage.getSpaceQuota());
verifyTypeQuotaAndConsume(new long[] {-1, -1, -1, ssQuota * 2, -1, -1}, null,
usage);
verifyTypeQuotaAndConsume(new long[] {-1, -1, ssQuota * 2, -1, -1, -1},
null, usage);
// Verify /type1 quota on NN1.
usage = client.getQuotaUsage("/type1");
assertEquals(HdfsConstants.QUOTA_RESET, usage.getQuota());
assertEquals(HdfsConstants.QUOTA_RESET, usage.getSpaceQuota());
verifyTypeQuotaAndConsume(new long[] {-1, -1, -1, ssQuota, -1, -1}, null,
verifyTypeQuotaAndConsume(new long[] {-1, -1, ssQuota, -1, -1, -1}, null,
usage);

FileSystem routerFs = routerContext.getFileSystem();
Expand All @@ -431,15 +431,15 @@ public void testStorageTypeQuota() throws Exception {
assertEquals(2, u1.getFileAndDirectoryCount());
assertEquals(HdfsConstants.QUOTA_RESET, u1.getSpaceQuota());
assertEquals(fileSize * 3, u1.getSpaceConsumed());
verifyTypeQuotaAndConsume(new long[] {-1, -1, -1, ssQuota, -1, -1},
new long[] {0, 0, 0, fileSize * 3, 0, 0}, u1);
verifyTypeQuotaAndConsume(new long[] {-1, -1, ssQuota, -1, -1, -1},
new long[] {0, 0, fileSize * 3, 0, 0, 0}, u1);
// Verify /type0 storage type quota usage on Router.
assertEquals(HdfsConstants.QUOTA_RESET, u0.getQuota());
assertEquals(4, u0.getFileAndDirectoryCount());
assertEquals(HdfsConstants.QUOTA_RESET, u0.getSpaceQuota());
assertEquals(fileSize * 3 * 2, u0.getSpaceConsumed());
verifyTypeQuotaAndConsume(new long[] {-1, -1, -1, ssQuota * 2, -1, -1},
new long[] {0, 0, 0, fileSize * 3 * 2, 0, 0}, u0);
verifyTypeQuotaAndConsume(new long[] {-1, -1, ssQuota * 2, -1, -1, -1},
new long[] {0, 0, fileSize * 3 * 2, 0, 0, 0}, u0);
}

@Test
Expand Down
7 changes: 7 additions & 0 deletions ...roject/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2431,6 +2431,10 @@ private void checkStoragePolicyEnabled(final String operationNameReadable,
* @throws IOException
*/
void setStoragePolicy(String src, String policyName) throws IOException {
if (policyName.equalsIgnoreCase(
HdfsConstants.ALLNVDIMM_STORAGE_POLICY_NAME)) {
requireEffectiveLayoutVersionForFeature(Feature.NVDIMM_SUPPORT);
}
final String operationName = "setStoragePolicy";
checkOperation(OperationCategory.WRITE);
checkStoragePolicyEnabled("set storage policy", true);
Expand Down Expand Up @@ -3571,6 +3575,9 @@ void setQuota(String src, long nsQuota, long ssQuota, StorageType type)
if (type != null) {
requireEffectiveLayoutVersionForFeature(Feature.QUOTA_BY_STORAGE_TYPE);
}
if (type == StorageType.NVDIMM) {
requireEffectiveLayoutVersionForFeature(Feature.NVDIMM_SUPPORT);
Copy link
Member

@ayushtkn ayushtkn Oct 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check should be done in case of setStoragePolicy also, if the storage policy is ALLNVDIMM

Copy link
Contributor

@huangtianhua huangtianhua Dec 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Please see code above. Thanks.

Copy link
Contributor

@huangtianhua huangtianhua Dec 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}
Comment on lines +3578 to +3580
Copy link
Contributor

@vinayakumarb vinayakumarb Oct 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar check you need to add when user tries to use NVDIMM based storage policy..

Copy link
Contributor

@huangtianhua huangtianhua Dec 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Please see code above. Thanks.

checkOperation(OperationCategory.WRITE);
final String operationName = getQuotaCommand(nsQuota, ssQuota);
final FSPermissionChecker pc = getPermissionChecker();
Expand Down
3 changes: 2 additions & 1 deletion ...doop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeLayoutVersion.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ public enum Feature implements LayoutFeature {
APPEND_NEW_BLOCK(-62, -61, "Support appending to new block"),
QUOTA_BY_STORAGE_TYPE(-63, -61, "Support quota for specific storage types"),
ERASURE_CODING(-64, -61, "Support erasure coding"),
EXPANDED_STRING_TABLE(-65, -61, "Support expanded string table in fsimage");
EXPANDED_STRING_TABLE(-65, -61, "Support expanded string table in fsimage"),
NVDIMM_SUPPORT(-66, -61, "Support NVDIMM storage type");

private final FeatureInfo info;

Expand Down
6 changes: 3 additions & 3 deletions ...hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestBlockStoragePolicy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1421,29 +1421,29 @@ public void testStorageType() {
final EnumMap<StorageType, Integer> map = new EnumMap<>(StorageType.class);

//put storage type is reversed order
map.put(StorageType.NVDIMM, 1);
map.put(StorageType.ARCHIVE, 1);
map.put(StorageType.DISK, 1);
map.put(StorageType.SSD, 1);
map.put(StorageType.RAM_DISK, 1);
map.put(StorageType.NVDIMM, 1);

{
final Iterator<StorageType> i = map.keySet().iterator();
Assert.assertEquals(StorageType.RAM_DISK, i.next());
Assert.assertEquals(StorageType.NVDIMM, i.next());
Assert.assertEquals(StorageType.SSD, i.next());
Assert.assertEquals(StorageType.DISK, i.next());
Assert.assertEquals(StorageType.ARCHIVE, i.next());
Assert.assertEquals(StorageType.NVDIMM, i.next());
}

{
final Iterator<Map.Entry<StorageType, Integer>> i
= map.entrySet().iterator();
Assert.assertEquals(StorageType.RAM_DISK, i.next().getKey());
Assert.assertEquals(StorageType.NVDIMM, i.next().getKey());
Assert.assertEquals(StorageType.SSD, i.next().getKey());
Assert.assertEquals(StorageType.DISK, i.next().getKey());
Assert.assertEquals(StorageType.ARCHIVE, i.next().getKey());
Assert.assertEquals(StorageType.NVDIMM, i.next().getKey());
}
}

Expand Down
3 changes: 2 additions & 1 deletion ...-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/protocol/TestLayoutVersion.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ public void testNameNodeFeatureMinimumCompatibleLayoutVersions() {
NameNodeLayoutVersion.Feature.APPEND_NEW_BLOCK,
NameNodeLayoutVersion.Feature.QUOTA_BY_STORAGE_TYPE,
NameNodeLayoutVersion.Feature.ERASURE_CODING,
NameNodeLayoutVersion.Feature.EXPANDED_STRING_TABLE);
NameNodeLayoutVersion.Feature.EXPANDED_STRING_TABLE,
NameNodeLayoutVersion.Feature.NVDIMM_SUPPORT);
for (LayoutFeature f : compatibleFeatures) {
assertEquals(String.format("Expected minimum compatible layout version " +
"%d for feature %s.", baseLV, f), baseLV,
Expand Down