Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,9 @@ public enum ResultCodes {

NOT_SUPPORTED_OPERATION,

PARTIAL_RENAME
PARTIAL_RENAME,

QUOTA_EXCEEDED

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -818,11 +818,11 @@ public void testTempMount() throws Exception {
VolumeArgs volumeArgs = new VolumeArgs.Builder()
.setAcls(Collections.singletonList(aclWorldAccess))
.setQuotaInCounts(1000)
.setQuotaInBytes("1MB").build();
.setQuotaInBytes("1TB").build();
// Sanity check
Assert.assertNull(volumeArgs.getOwner());
Assert.assertNull(volumeArgs.getAdmin());
Assert.assertEquals("1MB", volumeArgs.getQuotaInBytes());
Assert.assertEquals("1TB", volumeArgs.getQuotaInBytes());
Assert.assertEquals(1000, volumeArgs.getQuotaInCounts());
Assert.assertEquals(0, volumeArgs.getMetadata().size());
Assert.assertEquals(1, volumeArgs.getAcls().size());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -707,9 +707,71 @@ public void testPutKey() throws IOException {
}
}

@Test
public void testCheckUsedBytesQuota() throws IOException {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we add used bytes check in each test case?

String volumeName = UUID.randomUUID().toString();
String bucketName = UUID.randomUUID().toString();
OzoneVolume volume = null;

String value = "sample value";
int blockSize = (int) ozoneManager.getConfiguration().getStorageSize(
OZONE_SCM_BLOCK_SIZE, OZONE_SCM_BLOCK_SIZE_DEFAULT, StorageUnit.BYTES);
int valueLength = value.getBytes().length;
int countException = 0;

store.createVolume(volumeName);
volume = store.getVolume(volumeName);
// Set quota In Bytes for a smaller value
store.getVolume(volumeName).setQuota(
OzoneQuota.parseQuota("1 Bytes", 100));
volume.createBucket(bucketName);
OzoneBucket bucket = volume.getBucket(bucketName);

// Test write key.
// The remaining quota does not satisfy a block size, so the write fails.
try {
writeKey(bucket, UUID.randomUUID().toString(), ONE, value, valueLength);
} catch (IOException ex) {
countException++;
GenericTestUtils.assertExceptionContains("QUOTA_EXCEEDED", ex);
}
// Write failed, volume usedBytes should be 0
Assert.assertEquals(0L, store.getVolume(volumeName).getUsedBytes());

// Test write file.
// The remaining quota does not satisfy a block size, so the write fails.
try {
writeFile(bucket, UUID.randomUUID().toString(), ONE, value, 0);
} catch (IOException ex) {
countException++;
GenericTestUtils.assertExceptionContains("QUOTA_EXCEEDED", ex);
}
// Write failed, volume usedBytes should be 0
Assert.assertEquals(0L, store.getVolume(volumeName).getUsedBytes());

// Write large key, test allocateBlock fails.
store.getVolume(volumeName).setQuota(
OzoneQuota.parseQuota(blockSize + "Bytes", 100));
try {
OzoneOutputStream out = bucket.createKey(UUID.randomUUID().toString(),
valueLength, STAND_ALONE, ONE, new HashMap<>());
for(int i=0; i <= blockSize/value.length(); i++) {
out.write(value.getBytes());
}
out.close();
} catch (IOException ex) {
countException++;
GenericTestUtils.assertExceptionContains("QUOTA_EXCEEDED", ex);
}
// AllocateBlock failed, volume usedBytes should be 0
Assert.assertEquals(0L, store.getVolume(volumeName).getUsedBytes());

Assert.assertEquals(3, countException);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we add a multiple block write case here,so the first few block will succeed, while the later block will fail due to quota exceed?

}

@Test
@SuppressWarnings("methodlength")
public void testVolumeAndBucketUsedBytes() throws IOException {
public void testVolumeUsedBytes() throws IOException {
String volumeName = UUID.randomUUID().toString();
String bucketName = UUID.randomUUID().toString();
OzoneVolume volume = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,8 @@ enum Status {

PARTIAL_RENAME = 65;

QUOTA_EXCEEDED = 66;

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,16 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
.collect(Collectors.toList());
omKeyInfo.appendNewBlocks(newLocationList, false);

omVolumeArgs = getVolumeInfo(omMetadataManager, volumeName);
omBucketInfo = getBucketInfo(omMetadataManager, volumeName, bucketName);
// check volume quota
long preAllocatedSpace = newLocationList.size()
* ozoneManager.getScmBlockSize()
* omKeyInfo.getFactor().getNumber();
if (omVolumeArgs.getQuotaInBytes() > OzoneConsts.QUOTA_RESET) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we add a isQuotaInBytesSet check in OmVolumeArgs and use this function instead?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm going to put the if in checkVolumeQuotaInBytes, which might seem a little more compact.

checkVolumeQuotaInBytes(omVolumeArgs, preAllocatedSpace);
}

// Add to cache entry can be done outside of lock for this openKey.
// Even if bucket gets deleted, when commitKey we shall identify if
// bucket gets deleted.
Expand All @@ -290,13 +300,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
bucketName, Optional.absent(), Optional.of(missingParentInfos),
trxnLogIndex);

long scmBlockSize = ozoneManager.getScmBlockSize();
omVolumeArgs = getVolumeInfo(omMetadataManager, volumeName);
omBucketInfo = getBucketInfo(omMetadataManager, volumeName, bucketName);

// update usedBytes atomically.
long preAllocatedSpace = newLocationList.size() * scmBlockSize
* omKeyInfo.getFactor().getNumber();
omVolumeArgs.getUsedBytes().add(preAllocatedSpace);
omBucketInfo.getUsedBytes().add(preAllocatedSpace);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
getOmRequest());
OMClientResponse omClientResponse = null;

OmKeyInfo openKeyInfo;
OmKeyInfo openKeyInfo = null;
IOException exception = null;
OmVolumeArgs omVolumeArgs = null;
OmBucketInfo omBucketInfo = null;
Expand Down Expand Up @@ -192,9 +192,19 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
KEY_NOT_FOUND);
}

// Append new block

List<OmKeyLocationInfo> newLocationList = Collections.singletonList(
OmKeyLocationInfo.getFromProtobuf(blockLocation));
omVolumeArgs = getVolumeInfo(omMetadataManager, volumeName);
omBucketInfo = getBucketInfo(omMetadataManager, volumeName, bucketName);
// check volume quota
long preAllocatedSpace = newLocationList.size()
* ozoneManager.getScmBlockSize()
* openKeyInfo.getFactor().getNumber();
if (omVolumeArgs.getQuotaInBytes() > OzoneConsts.QUOTA_RESET) {
checkVolumeQuotaInBytes(omVolumeArgs, preAllocatedSpace);
}
// Append new block
openKeyInfo.appendNewBlocks(newLocationList, false);

// Set modification time.
Expand All @@ -208,12 +218,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
new CacheKey<>(openKeyName),
new CacheValue<>(Optional.of(openKeyInfo), trxnLogIndex));

long scmBlockSize = ozoneManager.getScmBlockSize();
omVolumeArgs = getVolumeInfo(omMetadataManager, volumeName);
omBucketInfo = getBucketInfo(omMetadataManager, volumeName, bucketName);
// update usedBytes atomically.
long preAllocatedSpace = newLocationList.size() * scmBlockSize
* openKeyInfo.getFactor().getNumber();
omVolumeArgs.getUsedBytes().add(preAllocatedSpace);
omBucketInfo.getUsedBytes().add(preAllocatedSpace);

Expand All @@ -227,6 +232,18 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
} catch (IOException ex) {
omMetrics.incNumBlockAllocateCallFails();
exception = ex;
if (exception.toString().contains(
OMException.ResultCodes.QUOTA_EXCEEDED.toString())) {
long keyAllocatedSpace = openKeyInfo.getLatestVersionLocations()
.getLocationListCount() * ozoneManager.getScmBlockSize()
* openKeyInfo.getFactor().getNumber();
// Update usedBytes atomically. ErrorOMResponse does not persist the DB,
// so we update the cache first. The next time another key is written,
// volume Args will be persisted to the DB.
// TODO: There is a delay in updating DB in this way, and if necessary
// we can modify the ErrorOMResponse to avoid it.
omVolumeArgs.getUsedBytes().add(-keyAllocatedSpace);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If A object has multiple blocks, and quota_exceeded exception is thrown out at the last block, shall we only deduct the block space size at this step?

}
omClientResponse = new OMAllocateBlockResponse(createErrorOMResponse(
omResponse, exception));
LOG.error("Allocate Block failed. Volume:{}, Bucket:{}, OpenKey:{}. " +
Expand All @@ -239,8 +256,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
auditLog(auditLogger, buildAuditMessage(OMAction.ALLOCATE_BLOCK, auditMap,
exception, getOmRequest().getUserInfo()));



return omClientResponse;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

unnessary blank line.

}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.google.common.base.Preconditions;
import org.apache.hadoop.ozone.OmUtils;
import org.apache.hadoop.ozone.OzoneAcl;
import org.apache.hadoop.ozone.OzoneConsts;
import org.apache.hadoop.ozone.om.OMConfigKeys;
import org.apache.hadoop.ozone.om.exceptions.OMException;
import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs;
Expand Down Expand Up @@ -286,30 +287,33 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
.collect(Collectors.toList());
omKeyInfo.appendNewBlocks(newLocationList, false);

// Add to cache entry can be done outside of lock for this openKey.
// Even if bucket gets deleted, when commitKey we shall identify if
// bucket gets deleted.
omMetadataManager.getOpenKeyTable().addCacheEntry(
new CacheKey<>(dbOpenKeyName),
new CacheValue<>(Optional.of(omKeyInfo), trxnLogIndex));

long scmBlockSize = ozoneManager.getScmBlockSize();
omVolumeArgs = getVolumeInfo(omMetadataManager, volumeName);
omBucketInfo = getBucketInfo(omMetadataManager, volumeName, bucketName);

// Here we refer to the implementation of HDFS:
// If the key size is 600MB, when createKey, keyLocationInfo in
// keyLocationList is 3, and the every pre-allocated block length is
// 256MB. If the number of factor is 3, the total pre-allocated block
// ize is 256MB * 3 * 3. We will allocate more 256MB * 3 * 3 - 600mb * 3
// = 504MB in advance, and we will subtract this part when we finally
// commitKey.
long preAllocatedSpace = newLocationList.size() * scmBlockSize
long preAllocatedSpace = newLocationList.size()
* ozoneManager.getScmBlockSize()
* omKeyInfo.getFactor().getNumber();
// check volume quota
if (omVolumeArgs.getQuotaInBytes() > OzoneConsts.QUOTA_RESET) {
checkVolumeQuotaInBytes(omVolumeArgs, preAllocatedSpace);
}

// Add to cache entry can be done outside of lock for this openKey.
// Even if bucket gets deleted, when commitKey we shall identify if
// bucket gets deleted.
omMetadataManager.getOpenKeyTable().addCacheEntry(
new CacheKey<>(dbOpenKeyName),
new CacheValue<>(Optional.of(omKeyInfo), trxnLogIndex));

omVolumeArgs.getUsedBytes().add(preAllocatedSpace);
omBucketInfo.getUsedBytes().add(preAllocatedSpace);


// Prepare response
omResponse.setCreateKeyResponse(CreateKeyResponse.newBuilder()
.setKeyInfo(omKeyInfo.getProtobuf())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,26 @@ protected FileEncryptionInfo getFileEncryptionInfo(KeyArgs keyArgs) {
return encryptionInfo;
}

/**
* Check volume quota in bytes.
* @param omVolumeArgs
* @param allocateSize
* @throws IOException
*/
protected void checkVolumeQuotaInBytes(OmVolumeArgs omVolumeArgs,
long allocateSize) throws IOException {
long usedBytes = omVolumeArgs.getUsedBytes().sum();
long quotaInBytes = omVolumeArgs.getQuotaInBytes();
if (quotaInBytes - usedBytes < allocateSize) {
throw new OMException("The DiskSpace quota of volume:"
+ omVolumeArgs.getVolume() + "exceeded: quotaInBytes: "
+ quotaInBytes + " Bytes but diskspace consumed: " + (usedBytes
+ allocateSize) + " Bytes.",
OMException.ResultCodes.QUOTA_EXCEEDED);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

unnessary empty line.

}

/**
* Check directory exists. If exists return true, else false.
* @param volumeName
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ public static void addVolumeToDB(String volumeName, String ownerName,
OmVolumeArgs omVolumeArgs =
OmVolumeArgs.newBuilder().setCreationTime(Time.now())
.setVolume(volumeName).setAdminName(ownerName)
.setQuotaInBytes(Long.MAX_VALUE)
.setOwnerName(ownerName).build();
omMetadataManager.getVolumeTable().put(
omMetadataManager.getVolumeKey(volumeName), omVolumeArgs);
Expand Down