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

Auto shard #69

Merged
merged 9 commits into from
Sep 27, 2022
Merged

Auto shard #69

merged 9 commits into from
Sep 27, 2022

Conversation

mixmix
Copy link
Member

@mixmix mixmix commented Sep 14, 2022

This is WIP

Problem: the meta-feed spec describes where feeds should be located in nibble-shards. We want to save users from having to figure this out, so we need a nice methods for this

I wasn't sure what we want the final API of this module should be. I'd like to discuss this, but for the moment I've just put this method at a placeholder location ssb.metafeeds.v1.findOrCreate(details, cb)

I'll raise question I've encountered as comments in code

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
api.js Show resolved Hide resolved
pick-shard.js Show resolved Hide resolved
test/api.js Outdated Show resolved Hide resolved
test/api.js Outdated Show resolved Hide resolved
@mixmix mixmix requested a review from staltz September 14, 2022 05:32
@mixmix
Copy link
Member Author

mixmix commented Sep 14, 2022

TODO : export some test-vectors for sharding (to make it easier for Go / rust butts to align implementation)

@staltz
Copy link
Member

staltz commented Sep 14, 2022

I wasn't sure what we want the final API of this module should be. I'd like to discuss this

I also don't know what the final API should be. Might want to check @arj03 for ideas too.

One idea is to provide only a few APIs: findOrCreate (which is your new v1 findOrCreate) and getOrCreateRoot (name can be bikeshedded) and branchStream, and then tuck away all the other APIs under a lowlevel namespace or something like that. With the intention that the README only has the high level APIs, and we keep the low level APIs undocumented, sort of like a back stage access, that we can officialize if they seem important enough.

@staltz
Copy link
Member

staltz commented Sep 14, 2022

You know what, we would vastly simplify our lives if we dropped the possibility that a content leaf is a metafeed too. It would make implementing these changes in ssb-meta-feeds easier, and it would vastly simplify our upcoming changes to ssb-replication-scheduler.

To clarify, the metafeeds tree structure v1 spec allows this (focus on chess and tictactoe):

graph TB;
  root --> v1
  v1 --> 7 & b & f
  7 --> games
  games --> chess & tictactoe
  f --> posts
  b --> abouts
Loading

What if we would change the spec so that tree structure v1 is rigid, like this:

graph TB;
  root --> v1
  v1 --> 7 & b & f
  7 --> chess & tictactoe
  f --> posts
  b --> abouts
Loading

We can always just make tree structure v2 if we find a compelling case for that.

@staltz
Copy link
Member

staltz commented Sep 14, 2022

@mixmix Another thing: remove the package-lock.json that was added

@mixmix
Copy link
Member Author

mixmix commented Sep 15, 2022

I like your suggestion about "flat" leaves. Then we can hide almost all the bullshit...... OH I remember I was keen to have the unique name !== purpose because I argued for "clustering" of feeds

e.g. I might want to have chess/games and chess/meetups be in the same shard, because:

  • EITHER I'm a chess player, in which case i want
    • only the games feed
    • and MAYBE I want to upgrade to have the meetups for in person gathering of chess players
  • OR I'm not a chess player in which case I want NEITHER feed.

So we could make unique name = purpose.split('/')[0]

I dunno....

@mixmix
Copy link
Member Author

mixmix commented Sep 15, 2022

I like your suggestion about "flat" leaves. Then we can hide almost all the bullshit...... API which lets you make meta sub-feeds etc. and you really have to go looking to find those API

@staltz
Copy link
Member

staltz commented Sep 15, 2022

flat leaves

Yeah, this is going to require a PR to ssb-meta-feeds-spec.

lookup.js Outdated Show resolved Hide resolved
api.js Outdated Show resolved Hide resolved
api.js Outdated Show resolved Hide resolved
api.js Outdated Show resolved Hide resolved
@mixmix mixmix changed the base branch from master to atomic_tests September 20, 2022 05:19
@staltz staltz marked this pull request as draft September 21, 2022 11:19
@staltz
Copy link
Member

staltz commented Sep 21, 2022

I converted this PR to "draft" because it needs to be rebased on master.

@mixmix mixmix changed the base branch from atomic_tests to atomic-tests-cb-persist September 27, 2022 00:56
@mixmix mixmix force-pushed the atomic-tests-cb-persist branch from 70527d5 to dad1d53 Compare September 27, 2022 01:08
commit fdf2c98
Merge: 1663678 fbe1e39
Author: mixmix <[email protected]>
Date:   Tue Sep 27 13:52:33 2022 +1300

    Merge branch 'auto-shard' of github.com:ssbc/ssb-meta-feeds into auto-shard

commit 1663678
Merge: fd31fb1 70527d5
Author: mixmix <[email protected]>
Date:   Tue Sep 27 13:52:11 2022 +1300

    Merge branch 'atomic-tests-cb-persist' into auto-shard

commit 70527d5
Merge: 375bd22 8c9f464
Author: mixmix <[email protected]>
Date:   Tue Sep 27 13:51:02 2022 +1300

    Merge branch 'atomic-tests-cb-testbot' into atomic-tests-cb-persist

commit 8c9f464
Merge: 8ef1d0e 0e7a624
Author: mixmix <[email protected]>
Date:   Tue Sep 27 13:49:38 2022 +1300

    Merge branch 'master' of github.com:ssbc/ssb-meta-feeds into atomic-tests-cb-testbot

commit fbe1e39
Author: Andre Staltz <[email protected]>
Date:   Wed Sep 21 14:18:03 2022 +0300

    remove a code comment from api.js

commit fe51f97
Author: Andre Staltz <[email protected]>
Date:   Wed Sep 21 14:12:15 2022 +0300

    remove a code comment from lookup.js

commit fd31fb1
Author: mixmix <[email protected]>
Date:   Wed Sep 21 16:18:40 2022 +1200

    tidy

commit 4cad7a8
Author: mixmix <[email protected]>
Date:   Wed Sep 21 14:13:56 2022 +1200

    cover recps case, update README

commit 8ef1d0e
Author: mixmix <[email protected]>
Date:   Wed Sep 21 12:25:43 2022 +1200

    use testbot everywhere, group non-atomic tests

commit de10af1
Merge: 4e5accf 375bd22
Author: mixmix <[email protected]>
Date:   Wed Sep 21 11:44:44 2022 +1200

    Merge branch 'atomic-tests-cb-persist' into auto-shard

commit 4e5accf
Author: mixmix <[email protected]>
Date:   Wed Sep 21 11:40:38 2022 +1200

    fixup

commit 375bd22
Author: mixmix <[email protected]>
Date:   Wed Sep 21 11:21:36 2022 +1200

    add persistence tests to api

commit 13a0f22
Author: Andre Staltz <[email protected]>
Date:   Tue Sep 20 11:15:33 2022 +0300

    revert promise tests to cb tests

commit 8267fa8
Author: mixmix <[email protected]>
Date:   Tue Sep 20 17:33:45 2022 +1200

    fixup

commit ed72484
Author: mixmix <[email protected]>
Date:   Tue Sep 20 17:28:01 2022 +1200

    split out Advanced API to unclutter the README

commit 4409661
Author: mixmix <[email protected]>
Date:   Tue Sep 20 17:18:29 2022 +1200

    fixups

commit 6d4313c
Author: mixmix <[email protected]>
Date:   Wed Sep 14 17:10:01 2022 +1200

    test that shard-feed is created in correct place

commit ba96d7f
Author: mixmix <[email protected]>
Date:   Wed Sep 14 16:44:11 2022 +1200

    add API which auto-shards

commit e4cb0c8
Author: mixmix <[email protected]>
Date:   Tue Sep 20 16:54:41 2022 +1200

    make tests atomic, add persistence tests
@mixmix mixmix marked this pull request as ready for review September 27, 2022 01:31
README.md Show resolved Hide resolved
api.js Show resolved Hide resolved
Base automatically changed from atomic-tests-cb-persist to master September 27, 2022 07:51
@staltz staltz linked an issue Sep 27, 2022 that may be closed by this pull request
api.js Show resolved Hide resolved
test/api.test.js Outdated Show resolved Hide resolved
test/api.test.js Outdated Show resolved Hide resolved
test/api.test.js Outdated Show resolved Hide resolved
test/api.test.js Outdated Show resolved Hide resolved
test/api.test.js Outdated Show resolved Hide resolved
test/api.test.js Outdated Show resolved Hide resolved
Copy link
Member

@staltz staltz left a comment

Choose a reason for hiding this comment

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

Roger to go

@staltz staltz merged commit b42ab8d into master Sep 27, 2022
@staltz staltz deleted the auto-shard branch September 27, 2022 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet