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

cmd, discovery, server: allow B to set MaxBroadcastPrice to filter acceptable O's stored in cache #933

Merged
merged 1 commit into from Jun 25, 2019

Conversation

kyriediculous
Copy link
Contributor

@kyriediculous kyriediculous commented Jun 21, 2019

What does this pull request do? Explain your changes. (required)
Allows a broadcaster to set a maximum price preference BroadcastPrice *MaxBroadcastPrice (not required) in the same format as #916 either by specifying the flags maxPricePerUnit and pixelsPerUnit on node startup, or through the CLI.

When orchestratorPool.getOrchestrators is called on the orchestrators cached in the DBcache, it will filter orchestrators based on this max price setting (if set).

Specific updates (required)

  • Added maxPricePerUnit flag to cmd/livepeer.go
  • Added setting maxPricePerUnit and pixelsPerUnit through broadcaster CLI config
  • Added a predicate function to orchestratorPool struct
  • Added a new constructor to create an orchestratorPool with a predicate function NewOrchestratorPoolWithPred
  • Adjusted DBOrchestratorPoolCache.getOrchestrators to initiate orchestratorPool with a predicate function through the new constructor NewOrchestratorPoolWithPred. The predicate function returns true when B's MaxBroadcastPrice isn't set, or is greater than Orchestrators PriceInfo
  • Adjusted exported variable in server package BroadcastPrice to be a struct of type MaxBroadcastPrice consisting out of a *big.Rat and sync.RWMutex

How did you test each of these updates (required)
Added unit tests (80.3 % coverage for discovery package)
Added test to ./test_args.sh for when flags are set , but incorrectly provided

Does this pull request close any open issues?
Fixes #906

Checklist:

  • README and other documentation updated
  • Node runs in OSX and devenv
  • All tests in ./test.sh pass

cmd/livepeer/livepeer.go Outdated Show resolved Hide resolved
cmd/livepeer/livepeer.go Outdated Show resolved Hide resolved
cmd/livepeer/livepeer.go Outdated Show resolved Hide resolved
discovery/discovery.go Outdated Show resolved Hide resolved
server/mediaserver.go Outdated Show resolved Hide resolved
server/webserver.go Outdated Show resolved Hide resolved
cmd/livepeer/livepeer.go Outdated Show resolved Hide resolved
server/webserver.go Outdated Show resolved Hide resolved
cmd/livepeer_cli/wizard_broadcast.go Outdated Show resolved Hide resolved
discovery/discovery_test.go Show resolved Hide resolved
server/broadcast.go Outdated Show resolved Hide resolved
server/webserver.go Outdated Show resolved Hide resolved
server/webserver.go Outdated Show resolved Hide resolved
cmd/livepeer/livepeer.go Outdated Show resolved Hide resolved
cmd/livepeer/livepeer.go Outdated Show resolved Hide resolved
server/webserver.go Show resolved Hide resolved
server/webserver.go Show resolved Hide resolved
Copy link
Member

@yondonfu yondonfu left a comment

Choose a reason for hiding this comment

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

LGTM after a rebase!

@kyriediculous kyriediculous merged commit 17701ca into master Jun 25, 2019
@kyriediculous kyriediculous deleted the nv/b-price-preference branch June 25, 2019 19:27
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.

MVP B price per pixel preference setting
3 participants