Skip to content

Commit 667f1e6

Browse files
committed
wait time fix
1 parent 9c0cf62 commit 667f1e6

2 files changed

Lines changed: 90 additions & 12 deletions

File tree

libp2p/protocols/service_discovery/registrar.nim

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,6 @@ proc waitingTime*(
184184
if waitDuration < prevWaitDuration - elapsedDuration:
185185
waitDuration = prevWaitDuration - elapsedDuration
186186

187-
waitDuration = min(discoConfig.advertExpiry, waitDuration)
188-
189187
return waitDuration
190188

191189
proc updateLowerBounds*(
@@ -488,6 +486,8 @@ proc registration*(disco: ServiceDiscovery, peerId: PeerId, inMsg: Message): Mes
488486

489487
return msg
490488

489+
tWait = min(max(disco.discoConfig.advertExpiry, ZeroDuration), tWait)
490+
491491
disco.registrar.updateLowerBounds(serviceId, ad, tWait, now)
492492

493493
var ticket = Ticket(

tests/libp2p/service_discovery/test_registrar.nim

Lines changed: 88 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -227,13 +227,11 @@ suite "Service Discovery Registrar - Waiting Time Calculation":
227227
check w > 500.seconds
228228

229229
suite "Service Discovery Registrar - advertExpiry cap":
230-
# The wait time must never exceed advertExpiry: a wait longer than the
231-
# advert's own lifetime is pointless and is clamped at the end of waitingTime.
232230

233-
test "waitingTime clamps formula-driven wait to advertExpiry":
231+
test "waitingTime does not cap a formula-driven wait at advertExpiry":
234232
let registrar = Registrar.new()
235233
# cache at capacity ⇒ occupancy = 100.0; with safetyParam = 1.0 the
236-
# uncapped w = 100 * 100.0 * 1.0 = 10000s ≫ advertExpiry (100s).
234+
# w = 100 * 100.0 * 1.0 = 10000s ≫ advertExpiry (100s).
237235
let discoConfig =
238236
ServiceDiscoveryConfig.new(advertExpiry = 100.secs, safetyParam = 1.0)
239237
let serviceId = makeServiceId()
@@ -245,9 +243,9 @@ suite "Service Discovery Registrar - advertExpiry cap":
245243

246244
let w = registrar.waitingTime(discoConfig, ad, 1000, serviceId, now)
247245

248-
check w == discoConfig.advertExpiry
246+
check w == 10000.secs
249247

250-
test "waitingTime clamps lower-bound wait to advertExpiry":
248+
test "waitingTime does not cap a lower-bound-driven wait at advertExpiry":
251249
let registrar = Registrar.new()
252250
let discoConfig = ServiceDiscoveryConfig.new() # advertExpiry = 900s (default)
253251
let serviceId = makeServiceId()
@@ -260,12 +258,12 @@ suite "Service Discovery Registrar - advertExpiry cap":
260258

261259
let w = registrar.waitingTime(discoConfig, ad, 1000, serviceId, now)
262260

263-
check w == discoConfig.advertExpiry
261+
check w == 100000.secs
264262

265-
test "waitingTime cap leaves a legitimately small wait untouched":
263+
test "waitingTime returns a legitimately small wait untouched":
266264
let registrar = Registrar.new()
267-
# Empty cache, safetyParam = 0.5 ⇒ uncapped w = 10000 * 0.5 = 5000s,
268-
# comfortably below advertExpiry (10000s) so the clamp is a no-op.
265+
# Empty cache, safetyParam = 0.5 ⇒ w = 10000 * 0.5 = 5000s, comfortably
266+
# below advertExpiry (10000s) either way.
269267
let discoConfig =
270268
ServiceDiscoveryConfig.new(advertExpiry = 10000.secs, safetyParam = 0.5)
271269
let serviceId = makeServiceId()
@@ -277,6 +275,86 @@ suite "Service Discovery Registrar - advertExpiry cap":
277275
check w < discoConfig.advertExpiry
278276
check w == 5000.secs
279277

278+
test "registration caps the offered tWaitFor at advertExpiry":
279+
let advertExpiry = 100.secs
280+
let conf = ServiceDiscoveryConfig.new(
281+
advertExpiry = advertExpiry, safetyParam = 1.0, advertCacheCap = 10
282+
)
283+
let disco = setupServiceDiscoveryNode(discoConfig = conf)
284+
let serviceName = "service"
285+
let serviceId = serviceName.hashServiceId()
286+
let advertiserKey = PrivateKey.random(rng()).get()
287+
let advertiserId = PeerId.init(advertiserKey).get()
288+
let adBytes = makeAdvertisement(serviceName, advertiserKey).encode().get()
289+
290+
# Saturate the cache: occupancy pins at 100.0, so w = 100*100*1.0 = 10000s.
291+
for i in 0 ..< 10:
292+
disco.registrar.cacheTimestamps[(peerId: randomPeerId(), seqNo: uint64(i))] =
293+
Moment.now()
294+
295+
let inMsg = kadprotobuf.Message(
296+
msgType: kadprotobuf.MessageType.register,
297+
key: serviceId,
298+
register: Opt.some(
299+
kadprotobuf.RegisterMessage(
300+
advertisement: adBytes,
301+
status: Opt.none(kadprotobuf.RegistrationStatus),
302+
ticket: Opt.none(Ticket),
303+
)
304+
),
305+
)
306+
let reply = disco.registration(advertiserId, inMsg).register.get()
307+
308+
check reply.status.get() == kadprotobuf.RegistrationStatus.Wait
309+
check reply.ticket.get().tWaitFor.get() == advertExpiry
310+
311+
test "sustained overload keeps offering advertExpiry-length waits across retries":
312+
let advertExpiry = 100.secs
313+
let registrationWindow = 10.secs
314+
let conf = ServiceDiscoveryConfig.new(
315+
advertExpiry = advertExpiry,
316+
safetyParam = 1.0,
317+
advertCacheCap = 10,
318+
registrationWindow = registrationWindow,
319+
)
320+
let disco = setupServiceDiscoveryNode(discoConfig = conf)
321+
let serviceName = "service"
322+
let serviceId = serviceName.hashServiceId()
323+
let advertiserKey = PrivateKey.random(rng()).get()
324+
let advertiserId = PeerId.init(advertiserKey).get()
325+
let adBytes = makeAdvertisement(serviceName, advertiserKey).encode().get()
326+
327+
for i in 0 ..< 10:
328+
disco.registrar.cacheTimestamps[(peerId: randomPeerId(), seqNo: uint64(i))] =
329+
Moment.now()
330+
331+
let firstAttemptTime =
332+
Moment.init((Moment.now() - advertExpiry).epochSeconds, Second)
333+
var retryTicket = Ticket(
334+
advertisement: adBytes,
335+
tInit: firstAttemptTime,
336+
tMod: firstAttemptTime,
337+
tWaitFor: advertExpiry,
338+
signature: Opt.none(seq[byte]),
339+
)
340+
check retryTicket.sign(disco.switch.peerInfo.privateKey).isOk()
341+
342+
let inMsg = kadprotobuf.Message(
343+
msgType: kadprotobuf.MessageType.register,
344+
key: serviceId,
345+
register: Opt.some(
346+
kadprotobuf.RegisterMessage(
347+
advertisement: adBytes,
348+
status: Opt.none(kadprotobuf.RegistrationStatus),
349+
ticket: Opt.some(retryTicket),
350+
)
351+
),
352+
)
353+
let reply = disco.registration(advertiserId, inMsg).register.get()
354+
355+
check reply.status.get() == kadprotobuf.RegistrationStatus.Wait
356+
check reply.ticket.get().tWaitFor.get() == advertExpiry
357+
280358
suite "Service Discovery Registrar - Lower Bound Enforcement":
281359
test "waitingTime enforces service lower bound when exists":
282360
let registrar = Registrar.new()

0 commit comments

Comments
 (0)