graph/checkpoint/redis: support redis checkpoint service #720
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #720 +/- ##
===================================================
+ Coverage 87.42525% 87.63550% +0.21024%
===================================================
Files 281 283 +2
Lines 36621 36993 +372
===================================================
+ Hits 32016 32419 +403
+ Misses 3006 2964 -42
- Partials 1599 1610 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
nomand-zc
changed the title
| }, nil | ||
| } | ||
|
|
||
| func (s *Saver) findCheckpointID(ctx context.Context, lineageID, checkpointNS, checkpointID string) (string, error) { |
There was a problem hiding this comment.
查找逻辑应该是没和 sqlite 那边对齐？假如用户传空 namespace 的话，Redis saver 永远找不到任何 checkpoint，sqlite 的话则是会查所有 namespace 下的相关信息
There was a problem hiding this comment.
这里做了一下处理， 感觉这里需要使用ns做强制隔离， 不然有可能graphA 拿到的是graphB的checkpoint
There was a problem hiding this comment.
redis的处理方式是将空NS当前成普通的value来处理，sqlite那边后面可以在单独提一个pr修复这个问题。
There was a problem hiding this comment.
假如我现在只是知道一个 checkpoint id，期望从所有 namespace 下查找呢？
There was a problem hiding this comment.
假如我现在只是知道一个 checkpoint id，期望从所有 namespace 下查找呢？
有什么场景需要用到这种吗？感觉拿到的不一定是对的吧，会出现上面提到的graphA有可能拿到的是graphB写入的结果
There was a problem hiding this comment.
规范的使用方式我理解应该是lineageID = sessionID， NS = 默认等于graphagent.name（多任务并行使用同一个graphagent时，用户可以在自己定义一个graphagent.name + 业务的任务id）。而NS在graphagent里面会做默认值的处理。进到graph的Execute中其实就不会是空值了。
There was a problem hiding this comment.
梳理了一下获取checkpoint的几种方式：
- lineage_id、namespace、checkpint_id都不为空 （查找指定id的记录）
- lineage_id、namespace 不为空 （查找指定ns下最新的一条记录）
- lineage_id 不为空 （查找任意ns下最新的一条记录）
- lineage_id、checkpint_id 不为空 （查找任意ns下指定id的记录）
1，2两种场景没有问题，找到的肯定是同一个graph写入的。
3，4场景都不能保证查找到的记录是当前graph写入的。 如果是其他graph写入的其实也是不能给当前graph使用的。
所以感觉这里和lineage_id一样强制校验ns不能为空，或者ns为空当一个正常的值处理，不需要特殊处理。
目前redis版是会在graphagent中执行Executer前做默认值处理，来确保ns不为空。如果用户非得用空ns来查，那就当一个正常值来处理。 而sqlite中支持的3，4我理解使用的时候应该是有问题的（有多个graphagent的情况下）
There was a problem hiding this comment.
我想起来了，namespace 为空的处理是当时调 examples/graph/checkpoint 和 examples/graph/interrupt 做的处理，这俩示例应该是默认的 namespace 是空，当时应该还没有很确定 namespace 要怎么用，如果明确不为空的语义，你看看示例要不要改一下
There was a problem hiding this comment.
我想起来了，namespace 为空的处理是当时调 examples/graph/checkpoint 和 examples/graph/interrupt 做的处理，这俩示例应该是默认的 namespace 是空，当时应该还没有很确定 namespace 要怎么用，如果明确不为空的语义，你看看示例要不要改一下
done
2 participants
No description provided.