Missing return type hints on new public functions

Low Severity

touch_keys and touch_l1_keys are new public functions missing -> None return type annotations. The project's AGENTS.md states "All functions and methods must have type hints for their arguments and return values," and the review style guide classifies missing type hints on public functions as error-level. Other methods in the same classes (e.g., clear) include explicit -> None.

 

^{Triggered by project rule: LMCache Code Review Style Guide}

^{Reviewed by Cursor Bugbot for commit f7b89b9. Configure here.}

We probably want to keep this as well. In the P2P scenario, the keys might be read by a remote peer.
(A potential design concern here is whether the P2P peer will cause the same problem as vLLM APC).

@ApostaC Thanks for your reminder, I have kept this place. (
The current implements will trigger touch both in retrieve and store, and then touch all keys at the end_session, this may have some redundancy, but it doesn't seem like a big problem.) WDYT.

@ApostaC By the way, as show in the picture, when mla is enabled and tp > 1, the keys in all worker is same but will trigger multi finished_read, the keys will be touched multiple times.

Is it more reasonable to only touch keys that non-temp and non-read-locked, I also updated this in current PR.

    for key in keys:
            ...
            if not entry.read_lock.is_locked():
                if entry.is_temporary:
                    # NOTE: temporary objects shouldn't have write-locks
                    need_to_free.append(entry.memory_obj)
                    need_to_free_keys.append(key)
                    del self._objects[key]
                else:
                    # only the following two conditions are met,
                    # trigger `on_keys_touched` to update the LRU order.
                    # 1. non-temporary: temporary keys will be deleted;
                    # 2. non-read-locked: read-locked keys will not be evicted.
                    touch_keys.append(key)

        ...
        for listener in self._registered_listeners:
            listener.on_l1_keys_read_finished(touch_keys)
            ...
        ...

This check can avoid hold lock and release

Quick question: can you explain why we don't touch the read-locked keys? The touch operation simply changes the "priority" of the keys in the eviction policy. I'm not sure why the key being locked or not impacts the touch operation.

@ApostaC The touchis to update the order of lru queue, but only keys without locks will be evicted. Therefore, for read-locaked keys, just wait for the last touch.

Without this change, trigger touch in each finish_read, when mla is enabled and tp > 1, the same keys will be touched tp times.
In my tests, tp=2, and the log is as follows:

@chunxiaozheng thanks for the explanation! I would suggest we keep the old code since it does not create foreseeable correctness issues, and the old code is quite easier to understand -- unless we see a huge performance penalty of touching the chunk multiple times. WDYT?

okay

A small nit here: naming the variable touch_key but actually calling on_l1_keys_read_finished is a bit weird.
(Still trying to understand why we need to introduce touch_keys here)

@ApostaC thanks for your suggestion, how about naming read_finished_keys?