Skip to content

Commit 484decf

Browse files
baranovskyalexandrbaranovskyalexandr
authored
SITES-41091 (#263)
* SITES-41091 * SITES-41091 * SITES-41091: fix lint errors * SITES-41091: fix after code review
1 parent db592e7 commit 484decf

3 files changed

Lines changed: 106 additions & 15 deletions

File tree

‎actions/lib/aem.js‎

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@ class AdminAPI {
3131
inflight = [];
3232
MAX_RETRIES = 3;
3333
RETRY_DELAY = 5000;
34+
/** Max number of pending jobs (queues + inflight). Keep low to avoid "noisy neighbor" effect. */
35+
MAX_PENDING_JOBS = 20;
36+
/** Poll interval for job status (ms). Avoid polling too frequently (e.g. once every 5 seconds). */
37+
JOB_STATUS_POLL_INTERVAL_MS = 5000;
38+
/** Resolvers for backpressure: call when pending drops below MAX_PENDING_JOBS */
39+
_backpressureResolvers = [];
3440

3541
constructor(
3642
{ org, site },
@@ -50,15 +56,47 @@ class AdminAPI {
5056
}
5157

5258
previewAndPublish(records, locale, batchNumber) {
53-
return new Promise((resolve) => {
59+
const pushAndReturnPromise = (resolve) => {
5460
this.previewQueue.push({ records, locale, batchNumber, resolve });
55-
});
61+
};
62+
return this._waitPendingBelowLimit()
63+
.then(() => new Promise(pushAndReturnPromise));
5664
}
5765

5866
unpublishAndDelete(records, locale, batchNumber) {
59-
return new Promise((resolve) => {
67+
const pushAndReturnPromise = (resolve) => {
6068
this.unpublishQueue.push({ records, locale, batchNumber, resolve });
61-
});
69+
};
70+
return this._waitPendingBelowLimit()
71+
.then(() => new Promise(pushAndReturnPromise));
72+
}
73+
74+
/**
75+
* Returns current count of pending work (queues + inflight).
76+
* Used to keep pending jobs below MAX_PENDING_JOBS and avoid 409/429.
77+
*/
78+
getPendingCount() {
79+
return this.previewQueue.length + this.publishQueue.length
80+
+ this.unpublishQueue.length + this.unpublishPreviewQueue.length
81+
+ this.inflight.length;
82+
}
83+
84+
/**
85+
* Resolves when pending count is below MAX_PENDING_JOBS (backpressure).
86+
*/
87+
async _waitPendingBelowLimit() {
88+
while (this.getPendingCount() >= this.MAX_PENDING_JOBS) {
89+
await new Promise((resolve) => {
90+
this._backpressureResolvers.push(resolve);
91+
});
92+
}
93+
}
94+
95+
_resolveBackpressure() {
96+
if (this.getPendingCount() >= this.MAX_PENDING_JOBS) return;
97+
const resolvers = this._backpressureResolvers;
98+
this._backpressureResolvers = [];
99+
resolvers.forEach((r) => r());
62100
}
63101

64102
async startProcessing() {
@@ -103,6 +141,7 @@ class AdminAPI {
103141
this.inflight.push(promise);
104142
promise.then(() => {
105143
this.inflight.splice(this.inflight.indexOf(promise), 1);
144+
this._resolveBackpressure();
106145

107146
if (this.queue.length > 0) {
108147
const publishes = [];
@@ -245,8 +284,8 @@ class AdminAPI {
245284
}
246285
}
247286

248-
// Wait for 2 seconds before the next status check
249-
await new Promise(resolve => setTimeout(resolve, 2000));
287+
// Wait for 5 seconds before the next status check (avoid high request rate)
288+
await new Promise(resolve => setTimeout(resolve, this.JOB_STATUS_POLL_INTERVAL_MS));
250289
}
251290
}
252291

@@ -466,6 +505,7 @@ class AdminAPI {
466505
if (this.onQueuesProcessed) {
467506
this.onQueuesProcessed();
468507
}
508+
this._resolveBackpressure();
469509
}
470510
}
471511

‎test/aem.test.js‎

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ jest.mock('../actions/utils', () => ({
1919

2020
describe('AdminAPI Optimized Tests', () => {
2121
let adminAPI;
22-
const context = { logger: { info: jest.fn(), error: jest.fn() } };
22+
const context = { logger: { info: jest.fn(), error: jest.fn(), debug: jest.fn() } };
2323

2424
beforeEach(() => {
2525
adminAPI = new AdminAPI(
@@ -44,17 +44,20 @@ describe('AdminAPI Optimized Tests', () => {
4444
});
4545

4646
test('should add record to previewQueue on previewAndPublish', async () => {
47-
const record = { path: '/test' };
48-
adminAPI.previewAndPublish(record);
49-
expect(adminAPI.previewQueue).toHaveLength(1);
47+
const records = [{ path: '/test' }];
48+
adminAPI.previewAndPublish(records, null, 1);
49+
// Backpressure resolves immediately when pending < MAX_PENDING_JOBS; then the batch is pushed. Allow microtasks to run.
50+
await Promise.resolve();
5051
await Promise.resolve();
52+
expect(adminAPI.previewQueue).toHaveLength(1);
5153
});
5254

5355
test('should add record to unpublishQueue on unpublishAndDelete', async () => {
54-
const record = { path: '/test' };
55-
adminAPI.unpublishAndDelete(record);
56-
expect(adminAPI.unpublishQueue).toHaveLength(1);
56+
const records = [{ path: '/test' }];
57+
adminAPI.unpublishAndDelete(records, null, 1);
5758
await Promise.resolve();
59+
await Promise.resolve();
60+
expect(adminAPI.unpublishQueue).toHaveLength(1);
5861
});
5962

6063
test('should start processing queues', async () => {
@@ -114,4 +117,52 @@ describe('AdminAPI Optimized Tests', () => {
114117
adminAPI.processQueues();
115118
expect(context.logger.info).toHaveBeenCalledWith('Queues: preview=0, publish=0, unpublish live=0, unpublish preview=1, inflight=0, in queue=0');
116119
});
120+
121+
describe('Qatar / bulk job serialization and rate limiting (409/429 fix)', () => {
122+
test('getPendingCount returns queues + inflight', () => {
123+
adminAPI.previewQueue.push({ records: [], locale: null, batchNumber: 1, resolve: jest.fn() });
124+
adminAPI.publishQueue.push({ records: [], locale: null, batchNumber: 2, resolve: jest.fn() });
125+
expect(adminAPI.getPendingCount()).toBe(2);
126+
});
127+
128+
test('MAX_PENDING_JOBS is 20', () => {
129+
expect(adminAPI.MAX_PENDING_JOBS).toBe(20);
130+
});
131+
132+
test('JOB_STATUS_POLL_INTERVAL_MS is 5000', () => {
133+
expect(adminAPI.JOB_STATUS_POLL_INTERVAL_MS).toBe(5000);
134+
});
135+
136+
test('previewAndPublish waits when pending >= MAX_PENDING_JOBS (backpressure)', async () => {
137+
// Fill up to MAX_PENDING_JOBS pending (all in queues; no inflight so we never resolve backpressure from completion)
138+
for (let i = 0; i < adminAPI.MAX_PENDING_JOBS; i++) {
139+
adminAPI.previewQueue.push({
140+
records: [{ path: `/p/${i}` }],
141+
locale: null,
142+
batchNumber: i + 1,
143+
resolve: jest.fn(),
144+
});
145+
}
146+
expect(adminAPI.getPendingCount()).toBe(adminAPI.MAX_PENDING_JOBS);
147+
148+
adminAPI.previewAndPublish([{ path: '/extra' }], null, 999);
149+
await Promise.resolve();
150+
await Promise.resolve();
151+
// Should not have added the extra batch yet (still waiting for backpressure)
152+
expect(adminAPI.previewQueue).toHaveLength(adminAPI.MAX_PENDING_JOBS);
153+
expect(adminAPI.previewQueue.every((b) => b.batchNumber !== 999)).toBe(true);
154+
155+
// Simulate one item leaving the queue so pending drops below MAX_PENDING_JOBS
156+
adminAPI.previewQueue.pop();
157+
adminAPI._resolveBackpressure();
158+
159+
await Promise.resolve();
160+
await Promise.resolve();
161+
// Now the waiting batch should have been pushed
162+
expect(adminAPI.getPendingCount()).toBeLessThanOrEqual(adminAPI.MAX_PENDING_JOBS);
163+
expect(adminAPI.previewQueue).toContainEqual(
164+
expect.objectContaining({ batchNumber: 999 }),
165+
);
166+
});
167+
});
117168
});

‎test/pdp-renderer.test.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ describe('pdp-renderer', () => {
161161
162162
<meta name="description" content="The Crown Summit Backpack is equal"><meta name="keywords" content="backpack, hiking, camping"><meta name="image" content="http://www.aemshop.net/media/catalog/product/m/b/mb03-black-0.jpg"><meta name="id" content="7"><meta name="sku" content="25-MB03"><meta name="__typename" content="SimpleProductView"><meta property="og:type" content="og:product">
163163
164-
<script type="application/ld+json">{"@context":"http://schema.org","@type":"Product","sku":"25-MB03","name":"Crown Summit Backpack","gtin":"","description":"The Crown Summit Backpack is equal","@id":"https://store.com/products/crown-summit-backpack/25-MB03","offers":[{"@type":"Offer","sku":"25-MB03","url":"https://store.com/products/crown-summit-backpack/25-MB03","availability":"https://schema.org/InStock","price":38,"priceCurrency":"USD","itemCondition":"https://schema.org/NewCondition"}],"image":"http://www.aemshop.net/media/catalog/product/m/b/mb03-black-0.jpg"}</script>
164+
<script type="application/ld+json">{"@context":"http://schema.org","@type":"Product","sku":"25-MB03","name":"Crown Summit Backpack","gtin":"","description":"The Crown Summit Backpack is equal","@id":"https://store.com/products/crown-summit-backpack/25-mb03","offers":[{"@type":"Offer","sku":"25-MB03","url":"https://store.com/products/crown-summit-backpack/25-mb03","availability":"https://schema.org/InStock","price":38,"priceCurrency":"USD","itemCondition":"https://schema.org/NewCondition"}],"image":"http://www.aemshop.net/media/catalog/product/m/b/mb03-black-0.jpg"}</script>
165165
</head>
166166
167167
<body>
@@ -410,7 +410,7 @@ describe('pdp-renderer', () => {
410410
expect(optionsHeader).toHaveLength(1);
411411

412412
const optionsContainer = optionsHeader.parent().next();
413-
expect(optionsContainer.find('li').length).toBeGreaterThan(0);
413+
expect(optionsContainer.find('li')).not.toHaveLength(0);
414414
});
415415
})
416416

0 commit comments

Comments
 (0)