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

Update members tangle calculation for exclusion spec #75

Merged
merged 7 commits into from
Mar 30, 2023

Conversation

Powersource
Copy link
Collaborator

@Powersource Powersource commented Mar 22, 2023

Fixes #69

Based on #65

@Powersource Powersource changed the base branch from master to exclude-member March 22, 2023 13:40
Base automatically changed from exclude-member to master March 22, 2023 14:07
@Powersource
Copy link
Collaborator Author

Was first thinking I could get the epoch by getting the latest epoch tangle and using the previous. But that might be an array. More accurate to look it up based on the current writeKey.

@Powersource
Copy link
Collaborator Author

Hmm I can't just get my own feed with the secret as the purpose though since someone else might've been the creator of the epoch init.

@Powersource Powersource marked this pull request as ready for review March 22, 2023 15:09
@Powersource Powersource requested review from staltz, mixmix and arj03 March 22, 2023 15:10
@Powersource
Copy link
Collaborator Author

Tested performance using the get-tangle-100-publishes test

With group, and epoch tangles:

getTangles: 17.845s

With group, epoch, and members tangles:

getTangles: 20.140s

@Powersource
Copy link
Collaborator Author

With only the group tangle it was

getTangles: 16.390s

Copy link
Member

@mixmix mixmix left a comment

Choose a reason for hiding this comment

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

All looks good. All my comments are suggestions/ optimisations (no 🔥 i.e. must change)

Nice one

function addSomeTangles(content, tangles, cb) {
if (tangles.length === 0) return cb(null, content)

const currTangle = tangles[0]

getTangle[currTangle](content.recps[0], (err, generatedTangle) => {
getTangle(server, currTangle, content.recps[0], (err, generatedTangle) => {
// prettier-ignore
if (err) return cb(clarify(err, 'Failed to get group tangle when adding group tangle to message'))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (err) return cb(clarify(err, 'Failed to get group tangle when adding group tangle to message'))
if (err) return cb(clarify(err, `Failed to get ${currTangle} tangle when adding group tangle to message`))

addSomeTangles(content, tangles, (err, content) => {
// prettier-ignore
if (err) return cb(clarify(err, 'failed to add tangles to content'))
addSomeTangles(content, tangles, (err, content) => {
Copy link
Member

Choose a reason for hiding this comment

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

This is sufficient:

  addSomeTangles(content, tangles, cb)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I know but I wanted more clarify since this gets run a lot

Copy link
Member

Choose a reason for hiding this comment

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

And new Error('CLARIFICATION', {cause: err}) is the new clarify(err, 'CLARIFICATION')

Comment on lines +39 to +48
pull.map((feed) =>
pull(
server.db.query(where(author(feed.id)), toPullStream()),
// get all first messages, since that's where the init would be
pull.take(1)
)
),
pull.flatten(),
// find the init
pull.filter((msg) => msg.value?.content?.type === 'group/init'),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pull.map((feed) =>
pull(
server.db.query(where(author(feed.id)), toPullStream()),
// get all first messages, since that's where the init would be
pull.take(1)
)
),
pull.flatten(),
// find the init
pull.filter((msg) => msg.value?.content?.type === 'group/init'),
pull.map((feed) =>
pull(
server.db.query(
where(and(author(feed.id), type('group/init'))),
toPullStream()
),
pull.filter(isInit)
pull.take(1)
)
),
pull.flatten(),

will require type and isInit to be imported

Copy link
Member

Choose a reason for hiding this comment

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

this is faster because we already have an index on type

if (tangle === 'members') {
// we find the id of the init msg for the current epoch
pull(
server.metafeeds.branchStream({ old: true, live: false }),
Copy link
Member

Choose a reason for hiding this comment

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

Question: does this return all feeds or just your meta-feed feeds? (I can't remember and the docs don't say!)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think you have to specify opts.root to only get from one person

// prettier-ignore
return cb(new Error(`get-tangle expects valid groupId, got: ${groupId}`))
}

Copy link
Member

Choose a reason for hiding this comment

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

In ssb-tribes I put a cache over the groups tangle https://github.com/ssbc/ssb-tribes/blob/master/lib/get-group-tangle.js#L12

I will perhaps look at adding that in another PR in the future.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The jitdb cache seems really fast after the first query

Comment on lines +129 to +133
const { where, and, toPullStream } = ssb.db.operators
pull(
ssb.db.query(where(and(tangleRoot(root))), toPullStream()),
pull.collect(cb)
)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const { where, and, toPullStream } = ssb.db.operators
pull(
ssb.db.query(where(and(tangleRoot(root))), toPullStream()),
pull.collect(cb)
)
const { where, toCallback } = ssb.db.operators
ssb.db.query(where(tangleRoot(root)), toCallback(cb))

Comment on lines +121 to +123

//console.time('getTangles')

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
//console.time('getTangles')

@@ -129,6 +140,8 @@ test(`get-tangle-${n}-publishes`, (t) => {
(err) => {
t.error(err, 'no error publishing')

//console.timeEnd('getTangles')
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
//console.timeEnd('getTangles')

@Powersource Powersource merged commit 9943ec7 into master Mar 30, 2023
@Powersource Powersource deleted the members-epoch branch March 30, 2023 15:50
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.

Fix members tangle calculation when reset on new epochs
3 participants