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

feat: add download stats info #323

Merged
merged 11 commits into from Jan 17, 2024
Merged

Conversation

MrGlp
Copy link
Contributor

@MrGlp MrGlp commented Jan 15, 2024

#218

Hope it can increase health attributes

The ecology of bt resources is not good. Most downloads do not have public IP addresses. Many resources may not be downloadable. The purpose of adding this attribute is to visualize the health of the currently downloaded resources. If the attributes are all 0, you can give up directly. Changed resources

===

Currently three attributes have been added

  1. The number of torrent node peers, which can be used as the attribute of the total number of users of the link
  2. ActivePeers should be the number of nodes that are connected to the current node and have uploading and downloading behaviors, as an active user attribute.
  3. Number of seed nodes with all current resources

It can be similar to bitcomet's total number of users, number of active users, and number of seed resources, which can describe the health of seeds to users.

===

  1. 增加属性
  2. 每次开启下载时候刷新这个属性
  3. 每次调用 Meta 获取资源属性时候刷新这个属性

可以看看还有什么需要修改注意的地方值得补充的,我在做一下测试,你可以先看看当前的改动有什么问题没有,测完了我同步发到这边,我们一起看看
我基于这个做了一些测试,可以很好的反应下载的健康情况,无效的torrent资源各项值都为0,或者后两者属性都为0,机器热门的资源三个属性数目都极高

==

@monkeyWie
Copy link
Member

stats的话我建议在FetcherMeta里加一个Stats any属性,以方便后续各个协议接入,以后http也可以把每个连接下载的进度和状态也放stats里

@MrGlp
Copy link
Contributor Author

MrGlp commented Jan 16, 2024

stats的话我建议在FetcherMeta里加一个Stats any属性,以方便后续各个协议接入,以后http也可以把每个连接下载的进度和状态也放stats里

Okay, I'll update it later

@MrGlp
Copy link
Contributor Author

MrGlp commented Jan 16, 2024

是否可以新增一个 base.Stats 直接存放统计的一些信息,直接绕过使用 any 类型断言这种 map 取值的方式
bt 和 http 里面描述健康的维度不太相同,可以直接用不同字段区分

// Stats for download
type Stats struct {
// http stats
// TODO
// bt stats
// health indicators of torrents, from large to small, ConnectedSeeders are also the key to the health of seed resources
TotalPeers int json:"totalPeers"
ActivePeers int json:"activePeers"
ConnectedSeeders int json:"connectedSeeders"
}

@MrGlp
Copy link
Contributor Author

MrGlp commented Jan 16, 2024

BT health indicators look good

Next, I can add http-related content. I think about this part and leave it to be added later. I don’t know what content is displayed by downloaders such as idm. I have a large number of users who know this. I can add it later. For reference comparison

You can take a look, there should be no problem with BT stats.
image

@monkeyWie
Copy link
Member

是否可以新增一个 base.Stats 直接存放统计的一些信息,直接绕过使用 any 类型断言这种 map 取值的方式 bt 和 http 里面描述健康的维度不太相同,可以直接用不同字段区分

// Stats for download type Stats struct { // http stats // TODO // bt stats // health indicators of torrents, from large to small, ConnectedSeeders are also the key to the health of seed resources TotalPeers int json:"totalPeers" ActivePeers int json:"activePeers" ConnectedSeeders int json:"connectedSeeders" }

这样主要是为了Downloader和Fetcher之间解耦,目前都是这样设计的,比如 Request里的extra,我之前尝试过用go泛型来处理,但是go的泛型做的太拉胯了搞不定,所以只能用any了

@MrGlp
Copy link
Contributor Author

MrGlp commented Jan 17, 2024

是否可以新增一个 base.Stats 直接存放统计的一些信息,直接绕过使用 any 类型断言这种 map 取值的方式 bt 和 http 里面描述健康的维度不太相同,可以直接用不同字段区分
// Stats for download type Stats struct { // http stats // TODO // bt stats // health indicators of torrents, from large to small, ConnectedSeeders are also the key to the health of seed resources TotalPeers int json:"totalPeers" ActivePeers int json:"activePeers" ConnectedSeeders int json:"connectedSeeders" }

这样主要是为了Downloader和Fetcher之间解耦,目前都是这样设计的,比如 Request里的extra,我之前尝试过用go泛型来处理,但是go的泛型做的太拉胯了搞不定,所以只能用any了

看到了那个类型,他的本质上应该是为了兼容多种入参可选,定义了 map 传参的方式

但是当前的这个 stats 我们用作返回的结果给出去,它的内容是很确定的,也需要使用 any 去存放么,比如这个对象,可以直接通过 Stats 接口返回出去,直接通过属性取出来,http 或者 bt 属性肯定是不一样的,如果有相同的维度就复用字段就行,比如 speed
如果改成 Stats Any 这种类型,取出去还需要再次断言,并且往里看注释,才能知道需要取出什么字段,是不是成本更高了,你觉得呢,你可以看下现有的,如果需要改成 any 那种方式我来兼容就行
image

@monkeyWie
Copy link
Member

我懂你意思,但是我看目前抽出来的这三个字段还是只适用于bt协议,并不通用呀,每个协议之间的stats数据都会差异很大,干脆就断言处理吧,golang好像也不支持多态,不然可以定个基类后续有通用的字段再放到基类里

@MrGlp
Copy link
Contributor Author

MrGlp commented Jan 17, 2024

ok 晚点我调整后测试完呼叫你我们再看看

@MrGlp
Copy link
Contributor Author

MrGlp commented Jan 17, 2024

image

@monkeyWie 这样如何呢

Comment on lines 82 to 88
type BtStats struct {
// bt stats
// health indicators of torrents, from large to small, ConnectedSeeders are also the key to the health of seed resources
TotalPeers int `json:"totalPeers"`
ActivePeers int `json:"activePeers"`
ConnectedSeeders int `json:"connectedSeeders"`
}
Copy link
Member

Choose a reason for hiding this comment

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

Move to pkg/protocol/bt/model.go

Comment on lines 91 to 95
type HttpStats struct {
// http stats
// health indicators of http
}

Copy link
Member

Choose a reason for hiding this comment

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

Move to pkg/protocol/http/model.go

@monkeyWie
Copy link
Member

嗯嗯 差不多了,再调整下包路径就行

@MrGlp MrGlp requested a review from monkeyWie January 17, 2024 12:48
@codecov-commenter
Copy link

codecov-commenter commented Jan 17, 2024

Codecov Report

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

Comparison is base (cc0f61b) 61.06% compared to head (fd835cb) 60.82%.

Files Patch % Lines
internal/protocol/bt/fetcher.go 0.00% 8 Missing ⚠️
internal/protocol/http/fetcher.go 0.00% 3 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #323      +/-   ##
==========================================
- Coverage   61.06%   60.82%   -0.24%     
==========================================
  Files          19       19              
  Lines        2866     2877      +11     
==========================================
  Hits         1750     1750              
- Misses        920      931      +11     
  Partials      196      196              

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

@monkeyWie
Copy link
Member

哦,还有个点得调整下,就是stats应该做成实时去获取的,不用去定时refresh,通过实现fetcher接口的Stats()方法拿就行了,就和Progress()那个定义一样,然后meta那里就不用放stats了

@monkeyWie monkeyWie changed the title Dc/feature/healthy feat: add download stats info Jan 17, 2024
@monkeyWie monkeyWie added the enhancement New feature or request label Jan 17, 2024
@MrGlp
Copy link
Contributor Author

MrGlp commented Jan 17, 2024

就是stats应该做成实时去获取的,不用去定时refresh,通过

ok

Req *base.Request `json:"req"`
Res *base.Resource `json:"res"`
Opts *base.Options `json:"opts"`
Stats any `json:"stats"`
Copy link
Member

Choose a reason for hiding this comment

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

这里可以不要了

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@MrGlp MrGlp requested a review from monkeyWie January 17, 2024 13:55
@monkeyWie monkeyWie merged commit 99f4e8c into GopeedLab:main Jan 17, 2024
2 checks passed
@monkeyWie
Copy link
Member

Nice!

@MrGlp
Copy link
Contributor Author

MrGlp commented Jan 17, 2024

如果是使用 downloader 进行开发的话
image

应该暂时还没有办法调用到这个 stats 接口吧,这里我后续

  1. 是不是我也可以给 downloader 实现这个 api 吧,目前只有这种方式可以拿到 fetcher 吧
  2. 并且可以将速度等指标的统计放进来?
    @monkeyWie

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

Successfully merging this pull request may close these issues.

None yet

4 participants