Skip to content
This repository was archived by the owner on Dec 20, 2024. It is now read-only.

Conversation

@starnop
Copy link
Contributor

@starnop starnop commented Jun 18, 2019

Signed-off-by: Starnop [email protected]

Ⅰ. Describe what this PR did

We store the piece md5s with json.Marsal(slice), and we will get piece md5 files like this

["803d361428809167be0719fec41e571b","4027139d441a8f58b5279148ce7d5edc","8451dcbc3e8b544049d911273d9fb22b","573fcda3bd0ead944600af51fb9fa756","7c927626b0c8f05afd07fd7f20b5cd27","f1fe2699daea7d24964794988b219181","43047f23461e00855695ceb159bdfa99","d37ce5323a523c5527df58748ae54d4f","497830f85e8a165bb85ef87a6be61f6d","bc4da60219956919de9bb06ab8b999d1","697611243967318765324df47e926ea1","1b06b3a3926741d2f85f8dd9542076bc","82975171731b83d9c82f491dffc5fd31ee40348a"]

It's unfriendly to read and most importantly, it's not compatible with the Java version which looks like this

b91e1c4ea54d7a0a4e91f8ed46e28fc6:4194304
04141c6f421572aa923b014a4ec798cd:4194304
633960487b27a932f25a804dd6a110c9:4194304
38288e3eef5f8b647a0c70f8f54707b0:4194304
5c000deb6b753b1c47de1555207e7abe:4194304
58c10c3cd081f06de18849eebfe408b9:4194304
414894070b733c7799e3cd3ec4f0d6f3:4194304
3b311b9dccf787422a65042802ab2651:4194304
89c9ad7f40a8174d023146396a8df0dd:4194304
f2c43d002eb8044a5746a940f6b5c9bb:4194304
d828bf80ba67d840ade2784e019d0d07:4194304
3c0967037e8976ea61f4aba09dc60a5d:4194304
78c9049ed2c470dad739266e9fd6f3ec:4194304
a313461e372ecbbaf395b8312ab25fd1:4194304
25d7ed6a5c04ae5f8f1b6957f42c2e6d:4194304
8e609e4ef90d4a84efb0bbab3656e14b:4194304
830d722d39fc790c93b5f80106e173a2:4194304
f387d2c1cfc84ce85e170933dacd7ee8:4194304
6a5a3d495e277af758b71350d584065c:4194304
0365fdf86fa792162b98a86c10edcd18:4194304
ef51f5dbba8249f390711d024f1f291b:4194304
27c0a37648ba6e3e19cbb7dd5df7003e:4194304
c6104122eda6cf1f3f2b6c7b40a9f40e:3872743
43d625fb79799bd17d4ce362864dc2de
0bc7b286fdc95c95628fce303e7dd439fc292ca7

So I update the way to store the piece md5s.

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@codecov-io
Copy link

codecov-io commented Jun 18, 2019

Codecov Report

Merging #616 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #616      +/-   ##
==========================================
+ Coverage   46.07%   46.14%   +0.06%     
==========================================
  Files          99       99              
  Lines        5853     5843      -10     
==========================================
- Hits         2697     2696       -1     
+ Misses       2930     2924       -6     
+ Partials      226      223       -3
Impacted Files Coverage Δ
supernode/daemon/mgr/cdn/file_meta_data.go 48.48% <100%> (+2.4%) ⬆️
supernode/daemon/mgr/cdn/cdn_util.go 15.38% <0%> (-11.29%) ⬇️
supernode/daemon/mgr/cdn/super_writer_util.go 71.69% <0%> (-0.53%) ⬇️
supernode/daemon/mgr/cdn/super_reader.go 0% <0%> (ø) ⬆️
supernode/daemon/mgr/progress/progress_util.go 21.42% <0%> (+0.33%) ⬆️
supernode/daemon/mgr/scheduler/manager.go 25.98% <0%> (+0.78%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1070562...8d4e48f. Read the comment docs.

pieceMD5s = strings.Split(string(bytes), "\n")

for _, v := range pieceMD5s {
logrus.Infof("success to read piece md5 file: %s", v)
Copy link
Member

Choose a reason for hiding this comment

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

It's better to log this one line with taskId on debug level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact, they are some code for test. So sorry for that. I have deleted them. 😢

Copy link
Member

@lowzj lowzj left a comment

Choose a reason for hiding this comment

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

LGTM

@lowzj lowzj merged commit bc5cc1d into dragonflyoss:master Jun 18, 2019
starnop pushed a commit to starnop/Dragonfly that referenced this pull request Nov 27, 2019
feature: store piece md5s with native bytes
inoc603 pushed a commit to inoc603/Dragonfly that referenced this pull request Dec 23, 2019
feature: store piece md5s with native bytes
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants