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

feat: allow actor state processor to run without leasing #178

Merged
merged 2 commits into from
Oct 30, 2020

Conversation

iand
Copy link
Contributor

@iand iand commented Oct 30, 2020

Leasing of actor states to process causes high contention on the visor_processing_actors
table since many instances are competing to lock rows to update their leases.

With this change, when --actorchange-lease is set to zero then the actor change processor
will not use leasing at all. Instead it assumes that it can work on any actor states
between the height ranges specified with --from and --to. It further subdivides that
height range by the number of workers started so each worker gets its own range of
heights to process.

Note that this requires manual coordination of height ranges allocated to each
instance of visor otherwise each instance may duplicate work performed by another
instance.

Note also that --to should be set to a reasonable value, typically under the current
height of the chain. Since the range specified with --to and --from is subdivided
evenly between workers a very large upper bound could result in some workers being
allocated ranges that have no data on the chain.

Leasing of actor states to process causes high contention on the visor_processing_actors
table since many instances are competing to lock rows to update their leases.

With this change, when --actorchange-lease is set to zero then the actor change processor
will not use leasing at all. Instead it assumes that it can work on any actor states
between the height ranges specified with --from and --to. It further subdivides that
height range by the number of workers started so each worker gets its own range of
heights to process.

Note that this requires manual coordination of height ranges allocated to each
instance of visor otherwise each instance may duplicate work performed by another
instance.

Note also that --to should be set to a reasonable value, typically under the current
height of the chain. Since the range specified with --to and --from is subdivided
evenly between workers a very large upper bound could result in some workers being
allocated ranges that have no data on the chain.
@iand iand requested a review from placer14 October 30, 2020 12:43
if heightFrom > heightTo {
return xerrors.Errorf("--from must not be greater than --to")
}

Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

Copy link
Contributor

@placer14 placer14 left a comment

Choose a reason for hiding this comment

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

Nice opt-in solution!

commands/run.go Outdated Show resolved Hide resolved
@frrist frrist added the kind/enhancement Improvement to an existing feature label Oct 30, 2020
@placer14 placer14 merged commit 3bbc8fc into master Oct 30, 2020
@placer14 placer14 deleted the iand/actor-nolease branch October 30, 2020 16:15
@placer14
Copy link
Contributor

Applies to #126.

placer14 added a commit that referenced this pull request Nov 4, 2020
 By Ian Davis (25) and others
 Via GitHub
 origin/master: (66 commits)
  feat: add changelog generator (#199)
  feat: disable leasing when gas outputs lease time is zero (#198)
  feat: add power actor claim extraction
  chore: Include RC releases in push docker images (#195)
  fix(metrics): export the completion and batch selection views (#197)
  fix: fix actor completion query
  chore: add metrics to leasing and work completion queries
  continue message parsing task when cbor parse fails (#185)
  perf: ensure processing updates always include height in criteria (#192)
  perf: include height restrictions in update clauses of leasing queries (#189)
  open lotusrepo lens read only
  feat: allow actor state processor to run without leasing (#178)
  feat: rpc reconnection on failure (#149)
  chore: add changelog (#150)
  polish: update miner processing logic
  feat: add dynamic panel creation based on tags (#159)
  don't codeql on every commit to pr. only on PR merge and cron (#168)
  track statechange persist duration
  feat: add dynamic panel creation based on tags
  Replace filecoin-ffi with stubs to simplify build process (#166)
  ...

Conflicts:
	commands/run.go

 It looks like you may be committing a merge.
 If this is not correct, please remove the file
	/Users/mg/work/pl/sentinel/.git/modules/sentinel-visor/MERGE_HEAD
 and try again.
func estimateCurrentEpoch() int64 {
return int64(time.Since(time.Date(2020, 8, 24, 22, 0, 0, 0, time.UTC)) / (30 * time.Second))
return int64(time.Since(mainnetGenesis) / (builtin.EpochDurationSeconds))
Copy link

Choose a reason for hiding this comment

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

func estimateCurrentEpoch() int64 {
return int64(time.Since(mainnetGenesis) / (builtin.EpochDurationSeconds * time.Second))
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you open an issue if you think there is a problem here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants