Skip to content
This repository has been archived by the owner on Feb 3, 2023. It is now read-only.

Link optimization #1607

Merged
merged 32 commits into from
Jul 26, 2019
Merged

Link optimization #1607

merged 32 commits into from
Jul 26, 2019

Conversation

StaticallyTypedAnxiety
Copy link
Contributor

PR summary

Leveraging some of the link processing to the DHT node

testing/benchmarking notes

( if any manual testing or benchmarking was/should be done, add notes and/or screenshots here )

followups

( any new tickets/concerns that were discovered or created during this work but aren't in scope for review here )

changelog

Please check one of the following, relating to the CHANGELOG-UNRELEASED.md

  • [ x] this is a code change that effects some consumer (e.g. zome developers) of holochain core so it is added to the CHANGELOG-UNRELEASED.md (linked above), with the format - summary of change [PR#1234](https://github.com/holochain/holochain-rust/pull/1234)
  • this is not a code change, or doesn't effect anyone outside holochain core development

@StaticallyTypedAnxiety StaticallyTypedAnxiety changed the title [WIP]Link optimization Link optimization Jul 22, 2019
@willemolding willemolding self-requested a review July 24, 2019 00:38
@StaticallyTypedAnxiety
Copy link
Contributor Author

StaticallyTypedAnxiety commented Jul 24, 2019

Fixing app_spec after merge

Edit : Fixed

@@ -99,6 +99,12 @@ pub async fn hold_link_workflow(
// 3. If valid store the entry in the local DHT shard
await!(add_link(&link_add, &context))?;
context.log(format!("debug/workflow/hold_link: added! {:?}", link));

//4. store link_add Entry
await!(hold_entry_workflow(&entry_with_header, context.clone()))?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just to clarify does this now mean that the nodes that hold the base also hold the link meta on the base and the link_add entry?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

For the link meta => so that we can get the headers
Link Add Entry => So that we can get the target

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this seems ok. It does mean that LinkAdd entries will be twice as dense in the DHT space when there is sharding so I guess this is a storage vs. retrieval time tradeoff.

Copy link
Collaborator

@lucksus lucksus left a comment

Choose a reason for hiding this comment

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

Looks good basically!
I few remarks below.

#[derive(Debug, Serialize, Deserialize, PartialEq, DefaultJson, Clone)]
pub struct GetLinkData
{
pub address : Address,
Copy link
Collaborator

Choose a reason for hiding this comment

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

is the the link base or the address of the LinkAdd?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

address of the link_add :) will be used to filter for the remove_link

core/src/workflows/hold_link.rs Outdated Show resolved Hide resolved
core/src/workflows/remove_link.rs Outdated Show resolved Hide resolved
@willemolding willemolding self-requested a review July 26, 2019 17:34
@StaticallyTypedAnxiety StaticallyTypedAnxiety merged commit 5a3e186 into develop Jul 26, 2019
@zippy zippy deleted the link-optimization branch October 4, 2019 18:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants