Skip to content
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

Fix issue of not refreshing lease state within blockBlob.upload operation. #2354

Merged
merged 2 commits into from
Jun 18, 2024
Merged

Fix issue of not refreshing lease state within blockBlob.upload operation. #2354

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
386283a
Fix issue of not refreshing lease state within blockBlob.upload opera…
EmmaZhu Feb 1, 2024
d7d6a30
Add change log
EmmaZhu Jun 18, 2024
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
28 changes: 15 additions & 13 deletions src/blob/persistence/LokiBlobMetadataStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,9 +326,9 @@ export default class LokiBlobMetadataStore
prefix === ""
? { name: { $gt: marker }, accountName: account }
: {
name: { $regex: `^${this.escapeRegex(prefix)}`, $gt: marker },
accountName: account
};
name: { $regex: `^${this.escapeRegex(prefix)}`, $gt: marker },
accountName: account
};

// Workaround for loki which will ignore $gt when providing $regex
const query2 = { name: { $gt: marker } };
Expand Down Expand Up @@ -750,10 +750,10 @@ export default class LokiBlobMetadataStore

const leaseTimeSeconds: number =
doc.properties.leaseState === Models.LeaseStateType.Breaking &&
doc.leaseBreakTime
doc.leaseBreakTime
? Math.round(
(doc.leaseBreakTime.getTime() - context.startTime!.getTime()) / 1000
)
(doc.leaseBreakTime.getTime() - context.startTime!.getTime()) / 1000
)
: 0;

coll.update(doc);
Expand Down Expand Up @@ -975,7 +975,7 @@ export default class LokiBlobMetadataStore
if (blobDoc) {
LeaseFactory.createLeaseState(new BlobLeaseAdapter(blobDoc), context)
.validate(new BlobWriteLeaseValidator(leaseAccessConditions))
.sync(new BlobLeaseSyncer(blob)); // Keep original blob lease
.sync(new BlobWriteLeaseSyncer(blob)); // Keep original blob lease

if (
blobDoc.properties !== undefined &&
Expand Down Expand Up @@ -1689,10 +1689,10 @@ export default class LokiBlobMetadataStore

const leaseTimeSeconds: number =
doc.properties.leaseState === Models.LeaseStateType.Breaking &&
doc.leaseBreakTime
doc.leaseBreakTime
? Math.round(
(doc.leaseBreakTime.getTime() - context.startTime!.getTime()) / 1000
)
(doc.leaseBreakTime.getTime() - context.startTime!.getTime()) / 1000
)
: 0;

coll.update(doc);
Expand Down Expand Up @@ -1919,7 +1919,7 @@ export default class LokiBlobMetadataStore
leaseBreakTime:
destBlob !== undefined ? destBlob.leaseBreakTime : undefined,
committedBlocksInOrder: sourceBlob.committedBlocksInOrder,
persistency: sourceBlob.persistency,
persistency: sourceBlob.persistency,
blobTags: options.blobTagsString === undefined ? undefined : getTagsFromString(options.blobTagsString, context.contextId!)
};

Expand Down Expand Up @@ -2106,7 +2106,7 @@ export default class LokiBlobMetadataStore
leaseBreakTime:
destBlob !== undefined ? destBlob.leaseBreakTime : undefined,
committedBlocksInOrder: sourceBlob.committedBlocksInOrder,
persistency: sourceBlob.persistency,
persistency: sourceBlob.persistency,
blobTags: options.blobTagsString === undefined ? undefined : getTagsFromString(options.blobTagsString, context.contextId!)
};

Expand Down Expand Up @@ -2334,8 +2334,9 @@ export default class LokiBlobMetadataStore
throw StorageErrorFactory.getBlobNotFound(context.contextId);
}

const lease = new BlobLeaseAdapter(doc);
new BlobWriteLeaseValidator(leaseAccessConditions).validate(
new BlobLeaseAdapter(doc),
lease,
context
);

Expand Down Expand Up @@ -2372,6 +2373,7 @@ export default class LokiBlobMetadataStore
}

const coll = this.db.getCollection(this.BLOBS_COLLECTION);
new BlobWriteLeaseSyncer(doc).sync(lease);
doc.committedBlocksInOrder = doc.committedBlocksInOrder || [];
doc.committedBlocksInOrder.push(block);
doc.properties.etag = newEtag();
Expand Down
34 changes: 17 additions & 17 deletions src/blob/persistence/SqlBlobMetadataStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@ import PageWithDelimiter from "./PageWithDelimiter";
import { getBlobTagsCount, getTagsFromString } from "../utils/utils";

// tslint:disable: max-classes-per-file
class ServicesModel extends Model {}
class ContainersModel extends Model {}
class BlobsModel extends Model {}
class BlocksModel extends Model {}
class ServicesModel extends Model { }
class ContainersModel extends Model { }
class BlobsModel extends Model { }
class BlocksModel extends Model { }
// class PagesModel extends Model {}

interface IBlobContentProperties {
Expand Down Expand Up @@ -1037,10 +1037,10 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore {
containerModel.properties.leaseState ===
Models.LeaseStateType.Breaking && containerModel.leaseBreakTime
? Math.round(
(containerModel.leaseBreakTime.getTime() -
context.startTime!.getTime()) /
1000
)
(containerModel.leaseBreakTime.getTime() -
context.startTime!.getTime()) /
1000
)
: 0;

return {
Expand Down Expand Up @@ -1162,7 +1162,7 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore {

LeaseFactory.createLeaseState(new BlobLeaseAdapter(blobModel), context)
.validate(new BlobWriteLeaseValidator(leaseAccessConditions))
.sync(new BlobLeaseSyncer(blob)); // Keep original blob lease;
.sync(new BlobWriteLeaseSyncer(blob)); // Keep original blob lease;

if (
blobModel.properties !== undefined &&
Expand Down Expand Up @@ -1729,15 +1729,15 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore {

// TODO: Return blobCommittedBlockCount for append blob

let responds = LeaseFactory.createLeaseState(
let responds = LeaseFactory.createLeaseState(
new BlobLeaseAdapter(blobModel),
context
)
.validate(new BlobReadLeaseValidator(leaseAccessConditions))
.sync(new BlobLeaseSyncer(blobModel));
return {
...responds,
properties : {
properties: {
...responds.properties,
tagCount: getBlobTagsCount(blobModel.blobTags),
},
Expand Down Expand Up @@ -2418,11 +2418,11 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore {

const leaseTimeSeconds: number =
lease.leaseState === Models.LeaseStateType.Breaking &&
lease.leaseBreakTime
lease.leaseBreakTime
? Math.round(
(lease.leaseBreakTime.getTime() - context.startTime!.getTime()) /
1000
)
(lease.leaseBreakTime.getTime() - context.startTime!.getTime()) /
1000
)
: 0;

await BlobsModel.update(this.convertLeaseToDbModel(lease), {
Expand Down Expand Up @@ -3407,8 +3407,8 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore {

if (!blobModel.isCommitted) {
throw StorageErrorFactory.getBlobNotFound(context.contextId);
}
}

LeaseFactory.createLeaseState(
new BlobLeaseAdapter(blobModel),
context
Expand Down
25 changes: 23 additions & 2 deletions tests/blob/apis/appendblob.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ describe("AppendBlobAPIs", () => {
});
} catch (err) {
assert.deepStrictEqual(
err.code,
err.code,
"MaxBlobSizeConditionNotMet");
assert.deepStrictEqual(err.statusCode, 412);

Expand All @@ -470,7 +470,7 @@ describe("AppendBlobAPIs", () => {
});
} catch (err) {
assert.deepStrictEqual(
err.code,
err.code,
"AppendPositionConditionNotMet");
assert.deepStrictEqual(err.statusCode, 412);
return;
Expand Down Expand Up @@ -565,4 +565,25 @@ describe("AppendBlobAPIs", () => {
}
assert.fail();
});

it("Append block should refresh lease state @loki", async () => {
await appendBlobClient.create();

const leaseId = "abcdefg";
const blobLeaseClient = await appendBlobClient.getBlobLeaseClient(leaseId);
await blobLeaseClient.acquireLease(20);

await sleep(20000);

await appendBlobClient.appendBlock("a", 1);

try {
await blobLeaseClient.renewLease();
assert.fail();
} catch (err) {
assert.deepStrictEqual(err.code, "LeaseIdMismatchWithLeaseOperation");
assert.deepStrictEqual(err.statusCode, 409);
return;
}
});
});
25 changes: 24 additions & 1 deletion tests/blob/apis/blockblob.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ import {
bodyToString,
EMULATOR_ACCOUNT_KEY,
EMULATOR_ACCOUNT_NAME,
getUniqueName
getUniqueName,
sleep
} from "../../testutils";

// Set true to enable debug log
Expand Down Expand Up @@ -68,6 +69,28 @@ describe("BlockBlobAPIs", () => {
await containerClient.delete();
});

it("Block blob upload should refresh lease state @loki @sql", async () => {
await blockBlobClient.upload('a', 1);

const leaseId = "abcdefg";
const blobLeaseClient = await blockBlobClient.getBlobLeaseClient(leaseId);
await blobLeaseClient.acquireLease(20);

// Waiting for 20 seconds for lease to expire
await sleep(20000);

await blockBlobClient.upload('b', 1);

try {
await blobLeaseClient.renewLease();
assert.fail();
}
catch (error) {
assert.deepStrictEqual(error.code, "LeaseIdMismatchWithLeaseOperation");
assert.deepStrictEqual(error.statusCode, 409);
}
});

it("upload with string body and default parameters @loki @sql", async () => {
const body: string = getUniqueName("randomstring");
const result_upload = await blockBlobClient.upload(body, body.length);
Expand Down
Loading