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

fix(node): notify fetch completion earlier to avoid being skipped #1737

Merged
merged 1 commit into from
May 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .github/workflows/memcheck.yml
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,16 @@ jobs:
fi
if: always()

- name: check there is no failed replication fetch
shell: bash
timeout-minutes: 1
run: |
if grep -r "failed to fetch" $NODE_DATA_PATH
then
echo "We find failed replication fetch"
exit 1
fi

- name: Upload payment wallet initialization log
uses: actions/upload-artifact@main
with:
Expand Down
2 changes: 1 addition & 1 deletion sn_client/src/register.rs
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ impl ClientRegister {
// any error here will result in a repayment of the register
// TODO: be smart about this and only pay for storage if we need to
Err(err) => {
debug!("Failed to fetch register: {err:?}");
debug!("Failed to get register: {err:?}");
debug!("Creating Register as it doesn't exist at {addr:?}!");
let cmd = RegisterCmd::Create {
register: self.register.clone(),
Expand Down
8 changes: 5 additions & 3 deletions sn_node/src/put_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ impl Node {
pub(crate) async fn validate_and_store_record(&self, record: Record) -> Result<CmdOk> {
let record_header = RecordHeader::from_record(&record)?;

// Notify replication_fetcher to mark the attempt as completed.
// Send the notification earlier to avoid it got skipped due to:
// the record becomes stored during the fetch because of other interleaved process.
self.network.notify_fetch_completed(record.key.clone());

match record_header.kind {
RecordKind::ChunkWithPayment => {
let record_key = record.key.clone();
Expand Down Expand Up @@ -89,9 +94,6 @@ impl Node {
record_key,
RecordType::NonChunk(content_hash),
);
} else {
// Notify replication_fetcher to mark the attempt as completed.
self.network.notify_fetch_completed(record.key.clone());
}
result
}
Expand Down
Loading