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

Metering test gives different results locally (for markm) vs under CI #3538

Closed
erights opened this issue Jul 26, 2021 · 2 comments · Fixed by #3546
Closed

Metering test gives different results locally (for markm) vs under CI #3538

erights opened this issue Jul 26, 2021 · 2 comments · Fixed by #3546
Assignees
Labels
bug Something isn't working metering charging for execution (was: package: tame-metering and transform-metering) SwingSet package: SwingSet

Comments

@erights
Copy link
Member

erights commented Jul 26, 2021

See #3537

@erights erights added the bug Something isn't working label Jul 26, 2021
@erights
Copy link
Member Author

erights commented Jul 26, 2021

Hi @warner are you the best assignee?

@warner
Copy link
Member

warner commented Jul 27, 2021

Since @kriskowal experienced something similar, I'm guessing this is a consequence of different versions of SES, which will cause different metering results. That one test (test-dynamic-vat-metered.js "meters decrement") is more sensitive to the precise value of the meter than I thought it'd be.

I have a plan to make it less sensitive. The current test works by creating a Meter with a capacity of 200k, then running a consume() function (measured as taking about 36500 computrons per execution) several times. The test puts a notification threshold at 100k, and expects the Meter to notify on the 3rd call, and expire/underflow on the 6th call.

The less-sensitive approach would:

  • set the meter to 1M, set the notify threshold to 0, run consume() twice, to measure the usage on the first and second calls (from my experiment, the first call consumes about 1.5% more than subsequent ones). Assert that the notifier does not fire.
  • create a new vat, with a new meter, whose capacity is set to FIRST+1.5SECOND, and threshold at 1.5SECOND
    • run consume() once, which ought to leave the meter at 1.5*SECOND: no notification, no underflow
    • run consume() a second time, which ought to leave the meter at 0.5*SECOND: notifier fires, no underflow
    • run consume() a third time, which should underflow

I initially had a test that simply reset the meter to a higher value (after measuring consume() once), but then I removed the "arbitrary reset" API (as well as the "increment" API) because those didn't seem like something that should be available in normal operation.

@warner warner added SwingSet package: SwingSet metering charging for execution (was: package: tame-metering and transform-metering) labels Jul 27, 2021
@warner warner added this to the Testnet: Metering Phase milestone Jul 27, 2021
warner added a commit that referenced this issue Jul 27, 2021
This changes test-dynamic-vat-metered.js to be more adaptive to changes in
the number of computrons consumed by the test function, by setting the
capacity and notification thresholds to be between small integral multiples
of a measured usage, instead of hard-coded values.

refs #3308
fixes #3538
warner added a commit that referenced this issue Jul 27, 2021
This changes test-dynamic-vat-metered.js to be more adaptive to changes in
the number of computrons consumed by the test function, by setting the
capacity and notification thresholds to be between small integral multiples
of a measured usage, instead of hard-coded values.

refs #3308
fixes #3538
warner added a commit that referenced this issue Jul 28, 2021
This changes test-dynamic-vat-metered.js to be more adaptive to changes in
the number of computrons consumed by the test function, by setting the
capacity and notification thresholds to be between small integral multiples
of a measured usage, instead of hard-coded values.

refs #3308
fixes #3538
warner added a commit that referenced this issue Jul 28, 2021
This changes test-dynamic-vat-metered.js to be more adaptive to changes in
the number of computrons consumed by the test function, by setting the
capacity and notification thresholds to be between small integral multiples
of a measured usage, instead of hard-coded values.

refs #3308
fixes #3538
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working metering charging for execution (was: package: tame-metering and transform-metering) SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants