-
Notifications
You must be signed in to change notification settings - Fork 37
Sleep mode #252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sleep mode #252
Conversation
Signed-off-by: irar2 <[email protected]>
Signed-off-by: irar2 <[email protected]>
Signed-off-by: irar2 <[email protected]>
Signed-off-by: irar2 <[email protected]>
pkg/kv-cache/block_cache.go
Outdated
| } | ||
|
|
||
| func (bc *blockCache) start(ctx context.Context) { | ||
| bc.logger.V(logging.TRACE).Info("Starting KV cache") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this log message is not supposed to be printed a lot, consider to report on INFO level
pkg/kv-cache/block_cache.go
Outdated
| } | ||
|
|
||
| func (bc *blockCache) discard() { | ||
| bc.logger.V(logging.TRACE).Info("Discarding KV cache") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same, maybe use INFO
pkg/kv-cache/block_cache.go
Outdated
| } | ||
|
|
||
| func (bc *blockCache) activate() { | ||
| bc.logger.V(logging.TRACE).Info("Activating KV cache") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
| "github.com/vmihailenco/msgpack/v5" | ||
| ) | ||
|
|
||
| func ParseKVEvent(parts [][]byte, expectedTopic string, expectedSeq uint64) ([]uint64, []uint64, bool) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe move this function to common.test_utils like CreateSub?
| } else if string(tags) == "kv_cache" { | ||
| wakeUpKVCache = true | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in case tags are not defined, both wakeUpWeights and wakeUpKVCache should be true
pkg/llm-d-inference-sim/server.go
Outdated
| // Activate the kv cache if either the sim is sleeping and the wake up tags are not | ||
| // "only weights", or the sim is not sleeping but the cache is still disabled | ||
| // and the tags are "only kv cache" | ||
| if s.config.EnableKVCache && (s.isSleeping && !wakeUpWeights) || (!s.isSleeping && s.kvCacheDisabled && wakeUpKVCache) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't it be s.config.EnableKVCache && (....)?
pkg/llm-d-inference-sim/simulator.go
Outdated
| // variable VLLM_SERVER_DEV_MODE | ||
| isInDevMode bool | ||
| // in sleep mode, and if woken up with weights only, kv cache is disabled | ||
| kvCacheDisabled bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a duplication of bloockCach.disabled?
Signed-off-by: irar2 <[email protected]>
Signed-off-by: irar2 <[email protected]>
mayabar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
|
/lgtm |
Closes #218