Skip to content

perf(uploadstore): use deletewithstamp instead and tag deletion case …#5032

Merged
istae merged 2 commits intomasterfrom
uploadstore-opt
Mar 6, 2025
Merged

perf(uploadstore): use deletewithstamp instead and tag deletion case …#5032
istae merged 2 commits intomasterfrom
uploadstore-opt

Conversation

@istae
Copy link
Contributor

@istae istae commented Mar 4, 2025

…cleanup

Checklist

  • I have read the coding guide.
  • My change requires a documentation update, and I have done it.
  • I have added tests to cover my changes.
  • I have filled out the description and linked the related issues.

Description

internal.Uploadstore now uses DeleteWithStamp as the regular Delete does an internal load of the stamp, which is unnecessary in this case.
Also, removed the assignment of zero as tagID to the uploadItem in the case that the tag is missing. The item eventually get removed when the chunk is synced, so there is no need to the extra Put operation.

Open API Spec Version Changes (if applicable)

Motivation and Context (Optional)

Related Issue (Optional)

Screenshots (if appropriate):

@istae istae marked this pull request as ready for review March 5, 2025 14:15
@istae istae requested a review from acha-bill as a code owner March 5, 2025 14:15
@martinconic
Copy link
Contributor

martinconic commented Mar 6, 2025

When testing deferred uploads and deleting the tag used( in the stage where the BMT is constructed), I then do NOT see any logs that show pushsync is pushing chunks. There my by some weird cases when testing deferred uploads and deleting in the process the tag that is used.

@martinconic
Copy link
Contributor

Whith the latest

When testing deferred uploads and deleting the tag used( in the stage where the BMT is constructed), I then do NOT see any logs that show pushsync is pushing chunks. There my by some weird cases when testing deferred uploads and deleting in the process the tag that is used.

With the latest change(delete dirty tag in Close) this seems to not happen anymore and works fine

@martinconic
Copy link
Contributor

martinconic commented Mar 6, 2025

Even if previous change seems to fix the issue, I get another one when testing:
When testing deferred uploads and deleting the tag used(after BMT was created and when pushsync is starded), I get this this error messages a lot:

"time"="2025-03-06 18:44:46.695144" "level"="debug" "logger"="node/pushsync" "msg"="could not push to peer" "chunk_address"="b2f6d03da2a45d60cb233c16cbe1ea3d00f1cd7b14c2a2a15aa672f50b8299cd" "id_address"="b2f6d03da2a45d60cb233c16cbe1ea3d00f1cd7b14c2a2a15aa672f50b8299cd" "peer_address"="97d97526816a0bd1e88bf4bad949c408ee4fd6476ff12ca60eb2b36f0360490f" "error"="new stream for peer 97d97526816a0bd1e88bf4bad949c408ee4fd6476ff12ca60eb2b36f0360490f: send headers: read message: stream reset"
"time"="2025-03-06 18:44:46.794904" "level"="debug" "logger"="node/pushsync" "msg"="could not push to peer" "chunk_address"="83bfbb2ed16b44bc53d7de30ec2610e508d4eaf92efba22d1be532c3c4438a17" "id_address"="83bfbb2ed16b44bc53d7de30ec2610e508d4eaf92efba22d1be532c3c4438a17" "peer_address"="97d97526816a0bd1e88bf4bad949c408ee4fd6476ff12ca60eb2b36f0360490f" "error"="new stream for peer 97d97526816a0bd1e88bf4bad949c408ee4fd6476ff12ca60eb2b36f0360490f: send headers: read message: stream reset"
"time"="2025-03-06 18:44:46.995955" "level"="debug" "logger"="node/pushsync" "msg"="could not push to peer" "chunk_address"="99b4c012fd47958dd19bf83134f1247051c344db887cdf5430450de425928cb1" "id_address"="99b4c012fd47958dd19bf83134f1247051c344db887cdf5430450de425928cb1" "peer_address"="97d97526816a0bd1e88bf4bad949c408ee4fd6476ff12ca60eb2b36f0360490f" "error"="new stream for peer 97d97526816a0bd1e88bf4bad949c408ee4fd6476ff12ca60eb2b36f0360490f: send headers: read message: stream reset"
"time"="2025-03-06 18:44:47.394951" "level"="debug" "logger"="node/pushsync" "msg"="could not push to peer" "chunk_address"="aed9ed3c4862c213cfe5d3bc721543c321402d7b1bed6cec0d01a50ba90e37b4" "id_address"="aed9ed3c4862c213cfe5d3bc721543c321402d7b1bed6cec0d01a50ba90e37b4" "peer_address"="97d97526816a0bd1e88bf4bad949c408ee4fd6476ff12ca60eb2b36f0360490f" "error"="new stream for peer 97d97526816a0bd1e88bf4bad949c408ee4fd6476ff12ca60eb2b36f0360490f: send headers: read message: stream reset"

The file seems to be uploaded in the end, I can download it, but the messages show until each chunk is sent

@istae
Copy link
Contributor Author

istae commented Mar 6, 2025

Even if previous change seems to fix the issue, I get another one when testing: When testing deferred uploads and deleting the tag used(after BMT was created and when pushsync is starded), I get this this error messages a lot:

"time"="2025-03-06 18:44:46.695144" "level"="debug" "logger"="node/pushsync" "msg"="could not push to peer" "chunk_address"="b2f6d03da2a45d60cb233c16cbe1ea3d00f1cd7b14c2a2a15aa672f50b8299cd" "id_address"="b2f6d03da2a45d60cb233c16cbe1ea3d00f1cd7b14c2a2a15aa672f50b8299cd" "peer_address"="97d97526816a0bd1e88bf4bad949c408ee4fd6476ff12ca60eb2b36f0360490f" "error"="new stream for peer 97d97526816a0bd1e88bf4bad949c408ee4fd6476ff12ca60eb2b36f0360490f: send headers: read message: stream reset"
"time"="2025-03-06 18:44:46.794904" "level"="debug" "logger"="node/pushsync" "msg"="could not push to peer" "chunk_address"="83bfbb2ed16b44bc53d7de30ec2610e508d4eaf92efba22d1be532c3c4438a17" "id_address"="83bfbb2ed16b44bc53d7de30ec2610e508d4eaf92efba22d1be532c3c4438a17" "peer_address"="97d97526816a0bd1e88bf4bad949c408ee4fd6476ff12ca60eb2b36f0360490f" "error"="new stream for peer 97d97526816a0bd1e88bf4bad949c408ee4fd6476ff12ca60eb2b36f0360490f: send headers: read message: stream reset"
"time"="2025-03-06 18:44:46.995955" "level"="debug" "logger"="node/pushsync" "msg"="could not push to peer" "chunk_address"="99b4c012fd47958dd19bf83134f1247051c344db887cdf5430450de425928cb1" "id_address"="99b4c012fd47958dd19bf83134f1247051c344db887cdf5430450de425928cb1" "peer_address"="97d97526816a0bd1e88bf4bad949c408ee4fd6476ff12ca60eb2b36f0360490f" "error"="new stream for peer 97d97526816a0bd1e88bf4bad949c408ee4fd6476ff12ca60eb2b36f0360490f: send headers: read message: stream reset"
"time"="2025-03-06 18:44:47.394951" "level"="debug" "logger"="node/pushsync" "msg"="could not push to peer" "chunk_address"="aed9ed3c4862c213cfe5d3bc721543c321402d7b1bed6cec0d01a50ba90e37b4" "id_address"="aed9ed3c4862c213cfe5d3bc721543c321402d7b1bed6cec0d01a50ba90e37b4" "peer_address"="97d97526816a0bd1e88bf4bad949c408ee4fd6476ff12ca60eb2b36f0360490f" "error"="new stream for peer 97d97526816a0bd1e88bf4bad949c408ee4fd6476ff12ca60eb2b36f0360490f: send headers: read message: stream reset"

The file seems to be uploaded in the end, I can download it, but the messages show until each chunk is sent

this is libp2p related though.
if all the chunks have finished syncing, and the tag info correctly shows that as well, then all is good

@istae istae merged commit aa4ec60 into master Mar 6, 2025
15 checks passed
@istae istae deleted the uploadstore-opt branch March 6, 2025 21:34
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.

4 participants