Skip to content
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

[Agent][Feat] Add remote file caching #139

Closed

Conversation

Bobholamovic
Copy link
Member

@Bobholamovic Bobholamovic commented Dec 11, 2023

RemoteFile增加可选的缓存以提升性能。缓存功能默认开启。

当前的naive方案要点如下:

  1. FileManager在构造时支持指定参数cache_remote_files以启用或关闭远程文件缓存功能。

  2. FileManager内部使用一个FileCacheManager对象,该对象负责为文件ID分配FileCache对象。FileCache对象被绑定到一个本地文件,支持文件读写、定时刷新等功能。

  3. FileCache对象有三种状态:

    • alive / not discarded,active:缓存已激活,代表缓存与远端数据已经同步,可以直接从缓存中取数据。
    • alive / not discarded,not active:缓存未激活,代表缓存文件存在,但其中的内容可能已经过期,需要与远端同步。
    • not alive / discarded, not active:缓存已销毁,代表缓存已经被废弃,不再可用。

    FileCache初始化后处于前两种状态中的一种。前两种状态之间可以相互转换,但alive / not discarded -> not alive / discarded是不可逆的。

  4. FileCache对象内部具有一个定时器,该定时器能够让缓存在超过一定时间后失效(active -> not active)。缓存失效后,将在下一次读取时自动更新,也可以调用FileCache的方法手动更新。

  5. FileManager创建远程文件时(通过create_file_from_pathcreate_remote_file_from_pathcreate_file_from_bytes),如果缓存功能开启,则将创建RemoteFileWithCache对象。具体而言,RemoteFileWithCache对象是带有缓存支持的RemoteFile,通过将一个FileCache绑定到一个RemoteFile得到。RemoteFileWithCache.read_contents方法调用时,首先尝试从缓存中获取数据,如果缓存不可用,则仍然从远端拉取数据。

  6. 当内存中不存在任何使用FileCache对象的对象时,该FileCache对象将被销毁(alive / not discarded -> not alive / discarded),从FileCacheManager中移除,并执行可能被委托的清理函数(如删除缓存文件)。

更多细节详见comments。

暂时想到一些后续可以继续优化的点:

  1. 当前由FileManager统一管理和分配文件资源,在必要时将创建好的文件资源交给FileCache使用。也就是说,FileCache只对文件具有读、写权限,不具备创建、删除文件的能力,如果将文件管理能力进一步下放给FileCache,那么可以做一些进一步的性能优化,例如在缓存失效时即时清理文件。
  2. 当前只有在FileCache不再被任何对象使用时,才触发缓存的销毁,而考虑到FileManager默认工作在auto-registering模式,“FileCache不再被任何对象使用”是很少发生的,除非用户手动unregister来放弃文件。在存在大量文件的场景中,控制缓存最大数量,采用LRU、LFU等caching策略或许可以获得更好的性能。
  3. 当前允许手动销毁缓存。不过,缓存被销毁后,已经绑定该缓存的RemoteFileWithCache对象不能得到通知,而是直接fall back成普通的不带缓存的RemoteFile。可考虑使用观察者模式和中介者模式重构FileCacheFileCacheManagerRemoteFileWithCache之间的关系,允许RemoteFileWithCache中的缓存信息被动态更新。
  4. 将资源释放与对象的生命周期绑定通常被视为不好的实践,可能导致代码的复杂性增加以及一些其它关键问题。后续可考虑更显式的缓存管理策略。这可能需要将FileCacheManager进一步中心化。
  5. 根据实际使用场景优化CacheFile使用的锁。

@Bobholamovic Bobholamovic marked this pull request as draft December 11, 2023 05:27
@Bobholamovic Bobholamovic requested review from sijunhe and removed request for sijunhe December 11, 2023 07:43
erniebot-agent/erniebot_agent/agents/base.py Outdated Show resolved Hide resolved
erniebot-agent/erniebot_agent/file_io/base.py Outdated Show resolved Hide resolved
erniebot-agent/erniebot_agent/file_io/factory.py Outdated Show resolved Hide resolved
erniebot-agent/erniebot_agent/file_io/factory.py Outdated Show resolved Hide resolved
erniebot-agent/erniebot_agent/file_io/factory.py Outdated Show resolved Hide resolved
erniebot-agent/erniebot_agent/file_io/file_registry.py Outdated Show resolved Hide resolved
erniebot-agent/erniebot_agent/file_io/file_registry.py Outdated Show resolved Hide resolved
@@ -178,7 +184,7 @@ def _get_default_headers(self) -> Dict[str, str]:

def _build_file_obj_from_dict(self, dict_: Dict[str, Any]) -> RemoteFile:
metadata: Dict[str, Any]
if "meta" in dict_:
if dict_.get("meta"):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

考虑meta为空字符串的情况

@Bobholamovic Bobholamovic marked this pull request as ready for review December 13, 2023 11:32
@codecov-commenter
Copy link

codecov-commenter commented Dec 20, 2023

Codecov Report

Attention: 127 lines in your changes are missing coverage. Please review.

Comparison is base (eb4069d) 61.83% compared to head (3d64dc5) 63.45%.

Files Patch % Lines
...t-agent/src/erniebot_agent/file_io/file_manager.py 62.59% 49 Missing ⚠️
...niebot-agent/src/erniebot_agent/file_io/caching.py 88.11% 24 Missing ⚠️
...ot-agent/src/erniebot_agent/file_io/remote_file.py 42.42% 19 Missing ⚠️
...-agent/src/erniebot_agent/file_io/file_registry.py 58.33% 10 Missing ⚠️
...niebot-agent/src/erniebot_agent/file_io/factory.py 60.00% 8 Missing ⚠️
erniebot-agent/src/erniebot_agent/utils/mixins.py 57.89% 8 Missing ⚠️
...niebot-agent/src/erniebot_agent/utils/temp_file.py 77.77% 4 Missing ⚠️
erniebot-agent/src/erniebot_agent/agents/base.py 83.33% 2 Missing ⚠️
...bot-agent/src/erniebot_agent/file_io/local_file.py 85.71% 2 Missing ⚠️
erniebot-agent/src/erniebot_agent/file_io/base.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #139      +/-   ##
===========================================
+ Coverage    61.83%   63.45%   +1.61%     
===========================================
  Files           56       58       +2     
  Lines         2780     3133     +353     
===========================================
+ Hits          1719     1988     +269     
- Misses        1061     1145      +84     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Bobholamovic Bobholamovic marked this pull request as draft December 21, 2023 18:55
@Bobholamovic
Copy link
Member Author

此PR已过期,目前与主分支差距过大,现关闭此PR。后续将基于新的主分支代码重新考虑方案。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants