server : evict checkpoints within min-step of each other

[aldehir]

Overview

Continuation of #24176 (review)

When creating a new checkpoint, evict any checkpoints that are within min-step of an earlier one.

Since we create two checkpoints towards the end of the prompt, it is possible that the last checkpoint evicts the penultimate checkpoint. To avoid this, I keep track of the task id in the checkpoint and only evict checkpoints created from prior tasks. Let me know if there is a better approach.

fixes #25023

Requirements

• I have read and agree with the contributing guidelines
• AI usage disclosure: Yes, for review.

[aldehir]

Hmm, turns out the near end checkpoint isn't being created if within min-step, which I don't believe was intended.

I also opened #25420 to address prefill issues.

I can roll these fixes into a single PR if desired.

[ggerganov]

I also opened #25420 to address prefill issues.

I think it's safe to merge #25420.

[ggerganov]

Hmm, turns out the near end checkpoint isn't being created if within min-step, which I don't believe was intended.

Think we just have to allow near_prompt_end and it should be good:

diff --git a/tools/server/server-context.cpp b/tools/server/server-context.cpp
index 17941d9e9..f477d7a3a 100644
--- a/tools/server/server-context.cpp
+++ b/tools/server/server-context.cpp
@@ -3506,7 +3506,10 @@ private:
                     do_checkpoint = do_checkpoint && !has_mtmd;
 
                     // no need to create checkpoints that are too close together, unless it's the last user message
-                    do_checkpoint = do_checkpoint && (slot.prompt.checkpoints.empty() || is_last_user_message || n_tokens_start > slot.prompt.checkpoints.back().n_tokens + params_base.checkpoint_min_step);
+                    do_checkpoint = do_checkpoint && (
+                            slot.prompt.checkpoints.empty() ||
+                            is_last_user_message || near_prompt_end ||
+                            n_tokens_start > slot.prompt.checkpoints.back().n_tokens + params_base.checkpoint_min_step);
                     SLT_DBG(slot, "main/do_checkpoint = %s, pos_min = %d, pos_max = %d\n", do_checkpoint ? "yes" : "no", pos_min, pos_max);
 
                     // note: we create the checkpoint before calling llama_decode(), so the current batch is not

Doing some testing.

[ali0une]

@aldehir did you see my comment with the clean diff?

isn't prevention at checkpoint creation time better than the id_task field + eviction loop in create_checkpoint()?

[ali0une]

@ggerganov @aldehir thanks for the work on this. I wanted to share some benchmark data that might help decide between the eviction approach and prevention at creation time.

I tested a prevention-based approach on a 29-request agent workload (branching conversations, sleep/wake, MTP). The numbers vs pre-bug baseline:

Metric	Pre-bug	Prevention
Old checkpoint evictions	157	43 (-72%)
"Looking for better prompt" events	15	5
Catastrophic resets	multiple	0

The approach combines the near_prompt_end bypass you suggested with a relaxed floor (checkpoint_min_step / 2) for is_last_user_message instead of a full bypass. This prevents clustered checkpoints at the source, so no id_task field or post-creation eviction loop is needed:

diff --git a/tools/server/server-context.cpp b/tools/server/server-context.cpp
--- a/tools/server/server-context.cpp
+++ b/tools/server/server-context.cpp
@@ -3522,12 +3522,15 @@ private:
                     // do not checkpoint after mtmd chunks
                     do_checkpoint = do_checkpoint && !has_mtmd;

-                    // no need to create checkpoints that are too close together, unless it's the last user message
-                    // apply a relaxed floor for is_last_user_message to prevent pathological clustering (#25023)
+                    // no need to create checkpoints that are too close together, unless it's the last user
+                    // message or we are near the end of the prompt (#25023)
+                    // apply a relaxed floor for is_last_user_message to prevent pathological clustering;
+                    // near_prompt_end fires unconditionally to ensure the critical end-of-prompt checkpoint exists
                     const int32_t checkpoint_floor = slot.prompt.checkpoints.empty()
                         ? 0
                         : slot.prompt.checkpoints.back().n_tokens + params_base.checkpoint_min_step / 2;
                     do_checkpoint = do_checkpoint && (slot.prompt.checkpoints.empty()
+                        || near_prompt_end
                         || (is_last_user_message && n_tokens_start > checkpoint_floor)
                         || n_tokens_start > slot.prompt.checkpoints.back().n_tokens + params_base.checkpoint_min_step);
                     SLT_DBG(slot, "main/do_checkpoint = %s, pos_min = %d, pos_max = %d\n", do_checkpoint ? "yes" : "no", pos_min, pos_max);

Full analysis : post-enhanced-fix-analyse.md
router-post-enhanced-fix.log

[aldehir]

@ali0une I'm merging this in now and will continue to look for improvements. To do that, however, I need to find a way to benchmark this.

Regarding your min-step / 2 approach, I don't believe this really addresses the issue of the min-step invariant being violated for all user message derived checkpoints except for the last. On consecutive requests in an ongoing conversation, it would instead collapse the distance to min-step / 2, which is not what we want.