cnpm / cnpmcore

Private NPM Registry for Enterprise
https://npmmirror.com
MIT License
607 stars 80 forks source link

删除文件逻辑的一些疑问 #593

Closed baxtergu closed 10 months ago

baxtergu commented 1 year ago

遇到一种极端情况:

可能在使用过程中在存储中产生了不一致的文件,或者某个包描述文件丢失了,如果用户想要强制同步来修复这一错误可能会遇到因为文件不存在导致的文件删除错误(比如 s3 deleteObject 如果指定的 key 不存在 会走 Promise.reject),进而无法删除成功。

还有一些特殊情况会导致调用 nfsClient.remove() 方法时的 key 为空,这也可能会导致文件删除异常,进而导致删除失败。

目前遇到的受影响的场景包括:npm unpublish,sync with forceSyncHistory: true。只要有脏数据可能就会失败。

可能因为我的 nfsClient 是自己实现的,但是里面的 remove 方法没做容错兜底。很好奇 oss sdk 在删除一个不存在的 key 的时候是报异常还是正常,会不会也有类似问题?

elrrrrrrr commented 1 year ago

按目前流程设计,unpublish 和 forceSyncHistory 都是对 registry manifest 的修改。

产物 tgz 不会做删除处理,如果有产物 oss 问题,一般通过其他控制台或者管理平台做删除处理。

baxtergu commented 1 year ago

按目前流程设计,unpublish 和 forceSyncHistory 都是对 registry manifest 的修改。

产物 tgz 不会做删除处理,如果有产物 oss 问题,一般通过其他控制台或者管理平台做删除处理。

@elrrrrrrr 了解,我这边出现一种情况就是 manifest 文件可能不存在或者内容错误导致删除失败的场景,这个比较极端,可能是在写入的时候出现了网络问题,或者程序异常中断导致的。这种情况如果想恢复这个包的数据可能就需要强制 unpublish 后重新同步,但是unpublish 又会去读取 manifest 里的版本信息,就死循环了。

我用一个取巧的方式解决这个问题:找一个环境同步一次同样的包,然后把 redis 里的 abbr key 复制粘贴到有 manifest 损坏的那个环境对应的redis里,执行 unpublish 动作就可以成功。😋

但是我想是不是在 cnpmcore 的服务上针对这种场景做一种特殊的删除逻辑,不从 oss 里读而是从数据库里读来实现删除?数据库里一般是准的,但是oss里的那个文件不一定

hezhengxu2018 commented 11 months ago

有一个enableStoreFullPackageVersionManifestsToDatabase的配置项,开启之后会把manifest文件的内容存在数据库里,不过读取的时候还是依旧从nfs上读取,并不会走数据库。不知道设置这个配置项最初的考虑是什么,不过应该只要稍做修改就能支持。所有的manifest都从数据库读取对数据库的压力是比较大的,可能也是最后没有这么做的原因。

baxtergu commented 11 months ago

有一个enableStoreFullPackageVersionManifestsToDatabase的配置项,开启之后会把manifest文件的内容存在数据库里,不过读取的时候还是依旧从nfs上读取,并不会走数据库。不知道设置这个配置项最初的考虑是什么,不过应该只要稍做修改就能支持。所有的manifest都从数据库读取对数据库的压力是比较大的,可能也是最后没有这么做的原因。

有 redis 缓存的情况下如果从数据库里读取应该还好。

目前遇到一个场景是 nfs 上的 manifest.json 里有的版本在 package_version 表里没有,导致重新 sync 的时候被认为版本已存在,不会重新同步补充进来。

elrrrrrrr commented 11 months ago

有 redis 缓存的情况下如果从数据库里读取应该还好。

不太建议直接用 db 来存储 manifest,manifest 的体积是不可控的,sql 性能会有很大影响。 例如 @npm-torg/public-test-package-2 这种测试库。

elrrrrrrr commented 11 months ago

可能也是最后没有这么做的原因。

@hezhengxu2018 😄 cnpmjs.org 最开始时 manifest 也是存储在 db 的,现在用单独的块存储来实现也算是我们的核心优化点之一

elrrrrrrr commented 11 months ago

我这边出现一种情况就是 manifest 文件可能不存在或者内容错误导致删除失败的场景,这个比较极端

@baxtergu 可以提供一个可以重现的单测吗?似乎你已经想到了解决方案 😇

baxtergu commented 11 months ago

我这边出现一种情况就是 manifest 文件可能不存在或者内容错误导致删除失败的场景,这个比较极端

@baxtergu 可以提供一个可以重现的单测吗?似乎你已经想到了解决方案 😇

我尝试详细描述下这个现象的表现

pkg-a 这个包,在 s3 里保存的 full_manifest.json 文件里有三个 versions:

{
  ...
  "versions": {
    "1.0.0":{},
    "1.1.0":{}
    "1.2.0":{}
  }
}

但是这个包在数据库中通过下面语句查询的结果

set @pkg_name = 'pkg-a';
set @pkg_scope = '';
select version from package_versions where package_id = (select package_id from packages where name=@pkg_name and scope=@pkg_scope);

只有下面两个

这种情况下执行 npm unpublish pkg-a --force 一次,再次运行上面的 sql 会发现 version 中只删除了一条 version 记录,version 表中有几个版本就需要几次 unpublish 命令才能全部删完。

当库中的版本数量为 0 的时候,再去执行 unpublish 会有两种结果,一种是不报错正常返回,另一种是报错返回,他们有区别:

{
  "_id": "pkg-a",
  "name": "pkg-a",
  "time": {
    "created": "2023-09-28T00:00:00.000Z",
    "modified": "2023-09-28T10:00:00.000Z",
    "unpublished": "2023-10-01T10:00:00.000Z"
  },
  "dist-tags": {}
}

上面这种情况下重新创建 sync 任务就可以解决版本缺失的问题

我感觉这个问题可能跟 forceSyncHistory 对一个包同时执行多次有关,是不是有执行动作顺序的一些特殊情况导致先同步了后被另一个task删掉的情况。

@elrrrrrrr

elrrrrrrr commented 10 months ago

这种情况下执行 npm unpublish pkg-a --force 一次,再次运行上面的 sql 会发现 version 中只删除了一条 version 记录,version 表中有几个版本就需要几次 unpublish 命令才能全部删完。

@baxtergu 目前只支持 unpublish 单个版本

是不是有执行动作顺序的一些特殊情况导致先同步了后被另一个task删掉的情况。 如果在包同步期间,执行 unpublish 操作可能会导致 sync 异常 ( 先 sync 后被删除 由于 db 表和 oss 存储是异步的,可能出现刚才提到的情况。

如果是公网包的话建议还是通过 forceSyncHistory 参数来确保和上游 registry 内容保持一致。 感觉可以在可以在写操作期间查一下 db 记录,看包是否正在进行同步,减少问题触发几率。

baxtergu commented 10 months ago

已解答,感谢 @elrrrrrrr