address reviewer comments

- replace get_kv_events with take_events for consistency with request.take_events()
- remove wayward comment

Signed-off-by: alec-flowers <[email protected]>

Author: alec-flowers

tests/v1/core/test_prefix_caching.py

@@ -763,7 +763,7 @@ def test_kv_cache_events(blocks_to_cache: int):
     # Allocate Blocks
     # Should see a single block stored event with a blocks_to_cache number of
     # block hashes
-    # get_kv_events should reset the kv_event_queue
+    # take_events should reset the kv_event_queue
     manager = KVCacheManager(
         make_kv_cache_config(block_size, num_blocks),
         max_model_len=8192,
@@ -775,7 +775,7 @@ def test_kv_cache_events(blocks_to_cache: int):
     num_tokens = block_size * blocks_to_cache
     req0 = make_request("0", list(range(num_tokens)))
     _ = manager.allocate_slots(req0, num_tokens)
-    events = manager.get_kv_events()
+    events = manager.take_events()
 
     block = events[-1]
     assert (len(block.block_hashes) == blocks_to_cache == len(
@@ -792,7 +792,7 @@ def test_kv_cache_events(blocks_to_cache: int):
     manager.free(req0)
     req1 = make_request("1", list(range(num_tokens)))
     _ = manager.allocate_slots(req1, num_tokens)
-    events = manager.get_kv_events()
+    events = manager.take_events()
 
     for blocks in events[:-1]:
         assert blocks.block_hashes[0] in stored_block_hash
@@ -805,7 +805,7 @@ def test_kv_cache_events(blocks_to_cache: int):
     # Should see a single all blocks cleared event
     manager.free(req1)
     manager.reset_prefix_cache()
-    events = manager.get_kv_events()
+    events = manager.take_events()
 
     assert isinstance(events[-1], AllBlocksCleared)
     assert len(manager.block_pool.cached_block_hash_to_block) == 0

vllm/v1/core/block_pool.py

@@ -313,8 +313,8 @@ def get_usage(self) -> float:
         """
         return 1.0 - (self.get_num_free_blocks() / self.num_gpu_blocks)
 
-    def extract_kv_events(self) -> list[KVCacheEvent]:
-        """Atomically extracts all events and clears the queue.
+    def take_events(self) -> list[KVCacheEvent]:
+        """Atomically takes all events and clears the queue.
         
         Returns:
             A list of KV cache events.

vllm/v1/core/kv_cache_manager.py

@@ -387,10 +387,10 @@ def free_block_hashes(self, request: Request) -> None:
         """
         self.req_to_block_hashes.pop(request.request_id, None)
 
-    def get_kv_events(self) -> list[KVCacheEvent]:
-        """Get the KV cache events.
+    def take_events(self) -> list[KVCacheEvent]:
+        """Take the KV cache events from the block pool.
 
         Returns:
             A list of KV cache events.
         """
-        return self.block_pool.extract_kv_events()
+        return self.block_pool.take_events()

vllm/v1/core/sched/scheduler.py

@@ -688,7 +688,7 @@ def update_from_output(
         engine_core_outputs = EngineCoreOutputs(
             outputs=outputs,
             scheduler_stats=self.make_stats(spec_decoding_stats),
-            kv_cache_events=self.kv_cache_manager.get_kv_events(),
+            kv_cache_events=self.kv_cache_manager.take_events(),
         )
         if self.include_finished_set:
             #TODO currently sending duplicates here, improve this

vllm/v1/engine/async_llm.py

@@ -134,8 +134,6 @@ def from_vllm_config(
 
         # FIXME(rob): refactor VllmConfig to include the StatLoggers
         # include StatLogger in the Oracle decision.
-
-        # TODO (alec) ask rob about this
         if stat_loggers is not None:
             raise ValueError("Custom StatLoggers are not yet supported on V1. "
                              "Explicitly set VLLM_USE_V1=0 to disable V1.")