-
Notifications
You must be signed in to change notification settings - Fork 87
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
feat: impl fast semver search #495
Conversation
New and updated dependency changes detected. Learn more about Socket for GitHub ↗︎
Footnotes |
private readonly bugVersionStore: BugVersionStore; | ||
|
||
async getBugVersion(): Promise<BugVersion | undefined> { | ||
// TODO performance problem, cache bugVersion and update with schedule |
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.
这里可能是一个热点,如果上线后实际存在需要额外增加缓存。
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.
先加一个进程 LRU?缓存一下1分钟,性能应该就不是问题了。
@Inject() | ||
private readonly logger: EggLogger; | ||
|
||
async fixPaddingVersion(id?: number): Promise<void> { |
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.
需要在业务低峰阶段调用此方法来订正数据,此方法会造成单进程 CPU 持续打满。
} | ||
let manifest; | ||
if (isFullManifests) { | ||
manifest = await this.distRepository.findPackageVersionManifest(pkgId, version); |
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.
存在一次额外的数据库访问,可以直接用过 fullname + version 的方式拼接出 nfs key。
Codecov Report
@@ Coverage Diff @@
## master #495 +/- ##
==========================================
- Coverage 97.45% 97.18% -0.27%
==========================================
Files 168 174 +6
Lines 15691 16182 +491
Branches 2018 2078 +60
==========================================
+ Hits 15291 15727 +436
- Misses 400 455 +55
|
$and: [ | ||
{ | ||
isPreRelease: { | ||
$lte: 0, |
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.
$eq 0 性能会不会更好些?
@Attribute(DataTypes.BOOLEAN) | ||
isPreRelease: boolean; | ||
|
||
static beforeCreate(instance) { |
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.
学习了。
ADD | ||
COLUMN `is_pre_release` tinyint(4) DEFAULT NULL COMMENT '是否是先行版本', | ||
ADD | ||
KEY `idx_pkg_id_is_pre_release_padding_version` (`package_id`, `padding_version`, `is_pre_release`, `version`); |
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.
索引为什么会加上 version?
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.
这样可以直接利用索引数据,避免回表,通过 explain sql 可以看到 using index
是最佳的效果。
} | ||
return { manifest, blockReason, pkg }; | ||
const fullname = getFullname(scope, name); | ||
const result = npa(`${fullname}@${spec}`); |
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.
希望未来 npa 不会成为性能瓶颈
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.
这个开销比较大的,好在前面有 cdn 缓存,不会高频使用。
@@ -107,7 +107,7 @@ export class PackageVersionFileController extends AbstractController { | |||
} | |||
return files; | |||
} | |||
const { manifest } = await this.packageManagerService.showPackageVersionManifest(scope, name, versionOrTag); | |||
const { manifest } = await this.packageManagerService.showPackageVersionManifest(scope, name, versionOrTag, false, 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.
加一下 range 的测试用例?
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.
补充一下子 range 相关测试用例,其他没问题了。
cc @hax |
@elrrrrrrr 也帮忙看看,今晚合并,然后在 r.cnpmjs.org 跑一下补数据,看看跑完历史数据要多久。 |
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.
+1
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.
+1
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.
+1
[skip ci] ## [3.28.0](v3.27.0...v3.28.0) (2023-06-06) ### Features * impl fast semver search ([#495](#495)) ([a7fd3a8](a7fd3a8))
cc @elrrrrrrr 参考。 |
[Success] ,耗时:950,047(ms) |
> follow #495 after supporting spec, adjust the parameter validation rules 1. 🆕 Add `Spec` validation rule, validating the spec by npa 2. 🛠️ Upgrade versionOrTag to versionSpec to support semver expressions, such as `^2.x || > 3.x` --------- > follow #495 支持 spec 后,调整参数校验规则 1. 🆕 新增 `Sepc` 校验规则,使用 npa 拼接包名进行验证 2. 🛠️ versionOrTag 升级为 versionSpec 支持 semver 表达式,例如 `^2.x || > 3.x`
Including SQL change.
close #134