aglinxinyuan commented on code in PR #6879:
URL: https://github.com/apache/texera/pull/6879#discussion_r3649239463
##########
common/auth/src/test/scala/org/apache/texera/auth/UserActivityTrackerSpec.scala:
##########
@@ -168,4 +173,202 @@ class UserActivityTrackerSpec extends AnyFlatSpec with
Matchers {
noException should be thrownBy tracker.evictStale()
}
+
+ it should "not create a cooldown entry for a null uid" in {
+ val recorder = new Recorder
+ val clock = new
AtomicReference[Instant](Instant.parse("2026-01-01T00:00:00Z"))
+ val tracker = makeTracker(Duration.ofMinutes(5), recorder, clock)
+
+ tracker.markActive(null)
+
+ tracker.cooldownSize shouldBe 0
+ }
+
+ it should "keep exactly one cooldown entry per uid across repeated calls" in
{
+ val recorder = new Recorder
+ val t0 = Instant.parse("2026-01-01T00:00:00Z")
+ val clock = new AtomicReference[Instant](t0)
+ val tracker = makeTracker(Duration.ofMinutes(5), recorder, clock)
+
+ tracker.markActive(42)
+ clock.set(t0.plus(Duration.ofMinutes(1)))
+ tracker.markActive(42)
+ clock.set(t0.plus(Duration.ofMinutes(6)))
+ tracker.markActive(42)
+
+ recorder.calls.size shouldBe 2
+ tracker.cooldownSize shouldBe 1
+ }
+
+ it should "stamp each upsert with the clock value at claim time" in {
+ val recorder = new Recorder
+ val t0 = Instant.parse("2026-01-01T00:00:00Z")
+ val t1 = t0.plus(Duration.ofMinutes(7))
+ val clock = new AtomicReference[Instant](t0)
+ val tracker = makeTracker(Duration.ofMinutes(5), recorder, clock)
+
+ tracker.markActive(42)
+ // a suppressed call in between must not change the recorded timestamps
+ clock.set(t0.plus(Duration.ofMinutes(3)))
+ tracker.markActive(42)
+ clock.set(t1)
+ tracker.markActive(42)
+
+ recorder.calls.poll() shouldBe ((42, t0))
+ recorder.calls.poll() shouldBe ((42, t1))
+ recorder.calls.size shouldBe 0
+ }
+
+ it should "evict only the stale entries and retain the fresh ones" in {
+ val recorder = new Recorder
+ val t0 = Instant.parse("2026-01-01T00:00:00Z")
+ val clock = new AtomicReference[Instant](t0)
+ val tracker = makeTracker(Duration.ofMinutes(5), recorder, clock)
+
+ tracker.markActive(1)
+ clock.set(t0.plus(Duration.ofMinutes(8)))
+ tracker.markActive(2)
+ tracker.cooldownSize shouldBe 2
+
+ // cutoff = t0 + 11min - 10min = t0 + 1min: uid 1 (t0) is stale, uid 2
(t0+8min) is not
+ clock.set(t0.plus(Duration.ofMinutes(11)))
+ tracker.evictStale()
+ tracker.cooldownSize shouldBe 1
+
+ // uid 2 is still in cooldown, so it must not produce a second write
+ tracker.markActive(2)
+ recorder.calls.size shouldBe 2
+ }
+
+ it should "retain an entry that is exactly 2 * writeInterval old" in {
+ val recorder = new Recorder
+ val t0 = Instant.parse("2026-01-01T00:00:00Z")
+ val clock = new AtomicReference[Instant](t0)
+ val tracker = makeTracker(Duration.ofMinutes(5), recorder, clock)
+
+ tracker.markActive(1)
+
+ // cutoff == t0 exactly; the boundary entry is not "before" the cutoff
+ clock.set(t0.plus(Duration.ofMinutes(10)))
+ tracker.evictStale()
+ tracker.cooldownSize shouldBe 1
+
+ // one millisecond past the boundary it is evicted
+ clock.set(t0.plus(Duration.ofMinutes(10)).plusMillis(1))
+ tracker.evictStale()
+ tracker.cooldownSize shouldBe 0
+ }
+
+ it should "tolerate evictStale on an empty tracker" in {
+ val recorder = new Recorder
+ val clock = new
AtomicReference[Instant](Instant.parse("2026-01-01T00:00:00Z"))
+ val tracker = makeTracker(Duration.ofMinutes(5), recorder, clock)
+
+ noException should be thrownBy tracker.evictStale()
+
+ tracker.cooldownSize shouldBe 0
+ recorder.calls.size shouldBe 0
+ }
+
+ it should "re-claim a uid whose entry was evicted" in {
+ val recorder = new Recorder
+ val t0 = Instant.parse("2026-01-01T00:00:00Z")
+ val clock = new AtomicReference[Instant](t0)
+ val tracker = makeTracker(Duration.ofMinutes(5), recorder, clock)
+
+ tracker.markActive(1)
+ clock.set(t0.plus(Duration.ofMinutes(11)))
+ tracker.evictStale()
+ tracker.cooldownSize shouldBe 0
+
+ tracker.markActive(1)
+
+ recorder.calls.size shouldBe 2
+ tracker.cooldownSize shouldBe 1
+ }
+
+ it should "perform exactly one upsert when many threads race on the same
uid" in {
+ val recorder = new Recorder
+ val clock = new
AtomicReference[Instant](Instant.parse("2026-01-01T00:00:00Z"))
+ val tracker = makeTracker(Duration.ofMinutes(5), recorder, clock)
+
+ val start = new CountDownLatch(1)
+ val threads = (1 to 16).map(_ =>
+ new Thread(() => {
+ start.await()
+ tracker.markActive(99)
+ })
+ )
+ threads.foreach(_.start())
+ start.countDown()
+ threads.foreach(_.join())
+
Review Comment:
Fixed — each thread now uses a 5s bounded join followed by
`withClue(s"thread $i did not finish within 5s: ")(t.isAlive shouldBe false)`,
so a regression fails fast and names the stuck thread instead of hanging the
suite. The existing concurrency assertions are unchanged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
