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

[Serverless] Add distributor action #467

Merged
merged 7 commits into from
Aug 31, 2021

Conversation

AlCutter
Copy link
Member

Update serverless witness actions to be a distributor

Part of #464

distributor on GitHub too.

For more details on witnessing, and the various roles involved (including
`distributor`), please see the [witness README](/google/trillian-examples/witness)
Copy link
Contributor

Choose a reason for hiding this comment

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

Fair warning: at least in the github preview this link does not work as expected - it takes me to https://github.com/google/trillian-examples/blob/master/google/trillian-examples/witness

Copy link
Member Author

Choose a reason for hiding this comment

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

hehe, too clever :)
done.

2. Initialise the distributor state:
1. we'll use a directory called `distributor` in our repo to
store the state files
2. TODO: config file
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this TODO for now, or a real TODO later?

Copy link
Member Author

Choose a reason for hiding this comment

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

I kinda TODONE it, linked to the example config

Now you can raise "incoming checkpoint" PRs which drop cosigned checkpoints into the
`distributor/logs/<log_id>/incoming` directory, whereupon the `witness_validator` action
should check the contents. Once the "incoming checkpoint" PRs are merged you
should see the `combine_witness_signs` action running in response (check the
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
should see the `combine_witness_signs` action running in response (check the
should see the `combine_witness_signatures` action running in response (check the

Copy link
Member Author

Choose a reason for hiding this comment

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

done


PRs containing cosigned checkpoint files under the log's `.../witness` directory are
PRs containing cosigned checkpoint files under the distributor's `logs/<logID>/incoming` directory are
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
PRs containing cosigned checkpoint files under the distributor's `logs/<logID>/incoming` directory are
PRs containing cosigned checkpoint files under the distributor's `distributor/logs/<logID>/incoming` directory are

There's a configurable directory that works as the root. Not sure if including it makes it more correct or just confusing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yes, I've used ellipsis elsewhere to suggest that this is internal structure to a defined directory, so I've added that here too - ta!

"golang.org/x/mod/sumdb/note"
)

// UpdateOpts contains the settings for the distributor update.
Copy link
Contributor

Choose a reason for hiding this comment

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

Something in this file needs to be clearer that while a distributor handles many logs, this update is processed/sharded per log.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, good call - modified the UpdateState comment to reflect that.

// Handle changes in number of required CPs
requiredSlots := int(opts.MaxWitnessSignatures + 1)
if l := len(state); l > requiredSlots {
state = state[0:requiredSlots]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
state = state[0:requiredSlots]
state = state[:requiredSlots]

Copy link
Member Author

Choose a reason for hiding this comment

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

done

ret[i] = nil
}
return ret, nil

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@AlCutter AlCutter merged commit 64f966c into google:master Aug 31, 2021
@AlCutter AlCutter deleted the serverless_distributor branch August 31, 2021 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes CLA signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants