Skip to content

Commit 3817224

Browse files
atamonatamon
authored
Merge pull request #32 from Yolean/last-seen-offsets-at-value-retrieval
Update last seen offset metric when getting values and perform leadin…
2 parents 198aacf + 8c1e75c commit 3817224

File tree

4 files changed

+102
-20
lines changed

4 files changed

+102
-20
lines changed

‎package-lock.json‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@yolean/kafka-keyvalue",
3-
"version": "1.5.0",
3+
"version": "1.6.1",
44
"keywords": [],
55
"author": "Yolean AB",
66
"license": "Apache-2.0",

‎src/KafkaKeyValue.spec.ts‎

Lines changed: 69 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ const promClientMock = {
3939
this.labels = jest.fn().mockReturnValue(this);
4040
this.reset = jest.fn();
4141
this.setToCurrentTime = jest.fn();
42-
this.startTimer = jest.fn();
42+
this.startTimer = jest.fn().mockReturnValue(() => jest.fn());
4343
this.remove = jest.fn();
4444
}
4545
},
@@ -54,7 +54,7 @@ const promClientMock = {
5454
constructor(options) {
5555

5656
this.observe = jest.fn();
57-
this.startTimer = jest.fn();
57+
this.startTimer = jest.fn().mockReturnValue(() => jest.fn());
5858
this.labels = jest.fn().mockReturnValue(this);
5959
this.reset = jest.fn();
6060
this.remove = jest.fn();
@@ -93,7 +93,6 @@ describe('KafkaKeyValue', function () {
9393
pixyHost: 'http://pixy',
9494
topicName: 'testtopic01',
9595
fetchImpl: fetchMock,
96-
updateDebounceTimeoutMs: 1
9796
});
9897

9998
const offset = await kkv.put('key1', 'value1');
@@ -118,7 +117,6 @@ describe('KafkaKeyValue', function () {
118117
pixyHost: 'http://pixy',
119118
topicName: 'testtopic01',
120119
fetchImpl: fetchMock,
121-
updateDebounceTimeoutMs: 1
122120
});
123121

124122
try {
@@ -171,6 +169,42 @@ describe('KafkaKeyValue', function () {
171169
expect(onValue).toBeCalledWith({ foo: 'bar' })
172170
expect(onValue).toBeCalledWith({ foo: 'bar2' })
173171
});
172+
173+
it('updates last seen offset metric based on header value', async function () {
174+
const response = {
175+
body: new EventEmitter(),
176+
headers: new Map([
177+
['x-kkv-last-seen-offsets', JSON.stringify([
178+
{ topic: 'testtopic01', partition: 0, offset: 17 }
179+
])]
180+
])
181+
};
182+
183+
const fetchMock = jest.fn().mockReturnValueOnce(response);
184+
185+
const metrics = KafkaKeyValue.createMetrics(promClientMock.Counter, promClientMock.Gauge, promClientMock.Histogram);
186+
const kkv = new KafkaKeyValue({
187+
cacheHost: 'http://cache-kkv',
188+
metrics,
189+
pixyHost: 'http://pixy',
190+
topicName: 'testtopic01',
191+
fetchImpl: fetchMock
192+
});
193+
194+
const streaming = kkv.streamValues(() => {});
195+
await Promise.resolve();
196+
response.body.emit('end');
197+
198+
await streaming;
199+
200+
expect(metrics.kafka_key_value_last_seen_offset.set).toHaveBeenCalledWith(
201+
{
202+
topic: 'testtopic01',
203+
partition: 0
204+
},
205+
17
206+
)
207+
});
174208
});
175209

176210
describe('onupdate handlers', function () {
@@ -183,7 +217,6 @@ describe('KafkaKeyValue', function () {
183217
metrics,
184218
pixyHost: 'http://pixy',
185219
topicName: 'testtopic01',
186-
updateDebounceTimeoutMs: 1
187220
});
188221

189222
const onUpdateSpy = jest.fn();
@@ -212,6 +245,22 @@ describe('KafkaKeyValue', function () {
212245
expect(metrics.kafka_key_value_last_seen_offset.labels).toHaveBeenCalledTimes(1);
213246
expect(metrics.kafka_key_value_last_seen_offset.labels).toHaveBeenCalledWith('cache-kkv', 'testtopic01', '0');
214247
expect(metrics.kafka_key_value_last_seen_offset.set).toHaveBeenCalledWith(28262);
248+
249+
updateEvents.emit('update', {
250+
v: 1,
251+
topic: 'testtopic01',
252+
offsets: {
253+
'0': 28263
254+
},
255+
updates: {
256+
'bd3f6188-d865-443d-8646-03e8f1c643cb': {}
257+
}
258+
});
259+
260+
// Promises needs to resolve before we get new value
261+
await new Promise(resolve => setTimeout(resolve, 10));
262+
263+
expect(onUpdateSpy).toHaveBeenCalledTimes(2);
215264
});
216265

217266
it('only handles updates for the same key once if called within the debounce timeout period', async function () {
@@ -221,7 +270,6 @@ describe('KafkaKeyValue', function () {
221270
metrics,
222271
pixyHost: 'http://pixy',
223272
topicName: 'testtopic01',
224-
updateDebounceTimeoutMs: 10
225273
});
226274

227275
const onUpdateSpy = jest.fn();
@@ -298,6 +346,21 @@ describe('KafkaKeyValue', function () {
298346
expect(onUpdateSpy).toHaveBeenCalledTimes(2);
299347
expect(onUpdateSpy).toHaveBeenCalledWith('bd3f6188-d865-443d-8646-03e8f1c643cb', { foo: 'bar' })
300348
expect(onUpdateSpy).toHaveBeenCalledWith('aaaa6188-d865-443d-8646-03e8f1c643cb', { foo: 'bar' })
349+
350+
updateEvents.emit('update', {
351+
v: 1,
352+
topic: 'testtopic01',
353+
offsets: {
354+
'0': 28265
355+
},
356+
updates: {
357+
'aaaa6188-d865-443d-8646-03e8f1c643cb': {}
358+
}
359+
});
360+
361+
await Promise.resolve();
362+
363+
expect(onUpdateSpy).toHaveBeenCalledTimes(3);
301364
});
302365
});
303366

@@ -309,7 +372,6 @@ describe('KafkaKeyValue', function () {
309372
metrics,
310373
pixyHost: 'http://pixy',
311374
topicName: 'testtopic01',
312-
updateDebounceTimeoutMs: 1
313375
});
314376

315377
kkv.updatePartitionOffsetMetrics({

‎src/KafkaKeyValue.ts‎

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const pGunzip = promisify<InputType, Buffer>(gunzip);
99
const pGzip = promisify<InputType, Buffer>(gzip);
1010

1111
const KKV_CACHE_HOST_READINESS_ENDPOINT = process.env.KKV_CACHE_HOST_READINESS_ENDPOINT || '/q/health/ready';
12-
const DEFAULT_UPDATE_DEBOUNCE_TIMEOUT_MS: number = 2000;
12+
const LAST_SEEN_OFFSETS_HEADER_NAME = 'x-kkv-last-seen-offsets';
1313

1414
export interface IKafkaKeyValueImpl { new (options: IKafkaKeyValue): KafkaKeyValue }
1515

@@ -20,7 +20,6 @@ export interface IKafkaKeyValue {
2020
gzip?: boolean
2121
metrics: IKafkaKeyValueMetrics
2222
fetchImpl?: IFetchImpl
23-
updateDebounceTimeoutMs?: number
2423
}
2524

2625
export interface CounterConstructor {
@@ -192,7 +191,7 @@ export default class KafkaKeyValue {
192191
private readonly metrics: IKafkaKeyValueMetrics;
193192
private readonly fetchImpl: IFetchImpl;
194193
private readonly logger;
195-
private readonly pendingKeyUpdates: Set<string> = new Set();
194+
private readonly lastKeyUpdate: Map<string, number> = new Map();
196195
private readonly partitionOffsets: Map<string, number> = new Map();
197196

198197
constructor(config: IKafkaKeyValue) {
@@ -202,7 +201,7 @@ export default class KafkaKeyValue {
202201
this.fetchImpl = getFetchImpl(config);
203202
this.logger = getLogger({ name: `kkv:${this.getCacheName()}` });
204203

205-
updateEvents.on('update', async (requestBody) => {
204+
updateEvents.on('update', async (requestBody: { v: number, offsets: { [partition: string]: number }, topic: string, updates: { [key: string]: {} } }) => {
206205
if (requestBody.v !== 1) throw new Error(`Unknown kkv onupdate protocol ${requestBody.v}!`);
207206

208207
const {
@@ -216,13 +215,20 @@ export default class KafkaKeyValue {
216215
return;
217216
}
218217

218+
const highestOffset: number = Object.values(offsets).reduce((memo, offset) => {
219+
return Math.max(memo, offset);
220+
}, -1);
221+
219222
if (this.updateHandlers.length > 0) {
220-
Object.keys(updates).forEach(key => this.pendingKeyUpdates.add(key));
221-
await new Promise(resolve => setTimeout(resolve, config.updateDebounceTimeoutMs || DEFAULT_UPDATE_DEBOUNCE_TIMEOUT_MS));
222223

223-
const updatesToPropagate = Array.from(this.pendingKeyUpdates).map(key => {
224-
this.pendingKeyUpdates.delete(key);
225-
return key;
224+
const updatesToPropagate: string[] = [];
225+
226+
Object.keys(updates).forEach(key => {
227+
const pendingOffset = this.lastKeyUpdate.get(key);
228+
if (pendingOffset === undefined || highestOffset > pendingOffset) {
229+
updatesToPropagate.push(key);
230+
this.lastKeyUpdate.set(key, highestOffset);
231+
}
226232
});
227233

228234
const updatesBeingPropagated = updatesToPropagate.map(async key => {
@@ -251,7 +257,7 @@ export default class KafkaKeyValue {
251257
updatePartitionOffsetMetrics(offsets: { [partition: string]: number }) {
252258
Object.entries(offsets).forEach(([partition, offset]) => {
253259
const existingOffset = this.partitionOffsets.get(partition);
254-
if (!existingOffset || existingOffset < offset) {
260+
if (existingOffset === undefined || existingOffset < offset) {
255261
this.partitionOffsets.set(partition, offset);
256262
this.metrics.kafka_key_value_last_seen_offset
257263
.labels(this.getCacheName(), this.topic, partition)
@@ -320,6 +326,9 @@ export default class KafkaKeyValue {
320326

321327
parseTiming();
322328
this.logger.debug({ key, value }, 'KafkaCache get value returned')
329+
330+
this.updateLastSeenOffsetsFromHeader(res);
331+
323332
return value;
324333
}
325334

@@ -336,6 +345,8 @@ export default class KafkaKeyValue {
336345

337346
streamTiming();
338347
this.logger.debug({ cache_name: this.getCacheName() }, 'Streaming values for cache finished');
348+
349+
this.updateLastSeenOffsetsFromHeader(res);
339350
}
340351

341352
async put(key: string, value: any, options: IRetryOptions = PUT_RETRY_DEFAULTS): Promise<number> {
@@ -353,4 +364,13 @@ export default class KafkaKeyValue {
353364
onUpdate(fn: UpdateHandler) {
354365
this.updateHandlers.push(fn);
355366
}
367+
368+
private updateLastSeenOffsetsFromHeader(res: Pick<Response, "headers">) {
369+
const lastSeenOffsetsHeader = res.headers.get(LAST_SEEN_OFFSETS_HEADER_NAME);
370+
if (!lastSeenOffsetsHeader) throw new Error(`Missing header "${LAST_SEEN_OFFSETS_HEADER_NAME}"`);
371+
const lastSeenOffsets = JSON.parse(lastSeenOffsetsHeader);
372+
lastSeenOffsets.forEach(({ topic, partition, offset }) => {
373+
this.metrics.kafka_key_value_last_seen_offset.set({ topic, partition }, offset);
374+
});
375+
}
356376
}

0 commit comments

Comments
 (0)