Revert "Enable Cross layers KV cache layout at NIXL Connector (#30207)"

[orozery]

Fully reverts #30207 and its #33052 follow-up.

[mergify]

Documentation preview: https://vllm--33241.org.readthedocs.build/en/33241/

[gemini-code-assist]

Code Review

This pull request reverts the "Cross layers KV cache layout" feature from the NIXL Connector, along with its follow-up. The changes correctly remove the feature's implementation, associated tests, and documentation. The code is reverted to its state before the feature was introduced, which also simplifies some logic by removing lazy initializations in favor of initialization in the constructor. The revert appears to be complete and correct.

[NickLucche]

Reverting as per discussion on slack.
Looking forward to get this feature back in on the next release!

[vadiklyutiy]

@orozery @liranschour don't we need cross layers kv-cache for performance reasons?

Seems #34158 and #36867 will solve the original issue.

[orozery]

@orozery @liranschour don't we need cross layers kv-cache for performance reasons?

Seems #34158 and #36867 will solve the original issue.

#33339 re-introduced it, but this time it is off by default.
It can be tested using:

--kv-transfer-config '{..., "kv_connector_extra_config": {"enable_cross_layers_blocks": "True"}}'

[vadiklyutiy]

#33339 re-introduced it, but this time it is off by default. It can be tested using:

--kv-transfer-config '{..., "kv_connector_extra_config": {"enable_cross_layers_blocks": "True"}}'

don't we want to enable it by default?

[vadiklyutiy]

and I see the following code

    @property
    def prefer_cross_layer_blocks(self) -> bool:
        backend = get_current_attn_backend(self._vllm_config)
        if backend.get_name() not in (
            "FLASH_ATTN",
            "FLASHINFER",
        ):
            return False

Don't know what problem with FLASH_ATTN. But we can enable it for FLASHINFER after 2 PRs mentioned above.

[orozery]

don't we want to enable it by default?

Eventually yes.
I think the plan is to to let users test it for a while before making it the default.

[vadiklyutiy]

don't we want to enable it by default?

Eventually yes. I think the plan is to to let users test it for a while before making it the default.

Just wondering, the change was merged 5 weeks ago. Did you get any positive or negative feedback?

[orozery]

Just wondering, the change was merged 5 weeks ago. Did you get any positive or negative feedback?

Good point.
@NickLucche we should make sure users are aware of it so we can get feedback.

[vadiklyutiy]

My concern about this approach

I think the plan is to to let users test it for a while before making it the default.

is that users aren't aware about the functionality and even if we tell about it to users who we are working directly, we will cover only minority of users who can get benefits from the functionality.