From 9c5624f9ddc53cb38abc0be7c21d8e9463245149 Mon Sep 17 00:00:00 2001 From: baoyachi Date: Tue, 5 Sep 2023 11:38:30 +0800 Subject: [PATCH 1/6] get Quota from RateLimiter --- governor/src/clock/with_std.rs | 2 +- governor/src/errors.rs | 4 +-- governor/src/gcra.rs | 38 +++++++++++++++++++++++++++++ governor/src/state.rs | 5 ++++ governor/src/state/direct/future.rs | 2 +- 5 files changed, 47 insertions(+), 4 deletions(-) diff --git a/governor/src/clock/with_std.rs b/governor/src/clock/with_std.rs index c7096046..df51d814 100644 --- a/governor/src/clock/with_std.rs +++ b/governor/src/clock/with_std.rs @@ -128,7 +128,7 @@ mod test { #[test] fn system_clock_impls_coverage() { let one_ns = Nanos::new(1); - let c = SystemClock::default(); + let c = SystemClock; let now = c.now(); assert_ne!(now + one_ns, now); // Thankfully, we're not comparing two system clock readings diff --git a/governor/src/errors.rs b/governor/src/errors.rs index 15872e5b..b5b33a7d 100644 --- a/governor/src/errors.rs +++ b/governor/src/errors.rs @@ -29,9 +29,9 @@ mod test { #[test] fn coverage() { let display_output = format!("{}", InsufficientCapacity(3)); - assert!(display_output.contains("3")); + assert!(display_output.contains('3')); let debug_output = format!("{:?}", InsufficientCapacity(3)); - assert!(debug_output.contains("3")); + assert!(debug_output.contains('3')); assert_eq!(InsufficientCapacity(3), InsufficientCapacity(3)); } } diff --git a/governor/src/gcra.rs b/governor/src/gcra.rs index aba2c195..7e698ff6 100644 --- a/governor/src/gcra.rs +++ b/governor/src/gcra.rs @@ -172,12 +172,22 @@ impl Gcra { } })) } + + /// Consumes the `Gcra` and returns `Quota`. + pub(crate) fn to_quota(&self) -> Quota { + let max_burst = self.tau / self.t; + Quota { + max_burst: NonZeroU32::new(max_burst as u32).expect("max_burst expect u32 value"), + replenish_1_per: Duration::from_nanos(self.t.as_u64()), + } + } } #[cfg(test)] mod test { use super::*; use crate::Quota; + use nonzero_ext::nonzero; use std::num::NonZeroU32; use proptest::prelude::*; @@ -222,6 +232,7 @@ mod test { #[derive(Debug)] struct Count(NonZeroU32); + impl Arbitrary for Count { type Parameters = (); fn arbitrary_with(_args: ()) -> Self::Strategy { @@ -251,4 +262,31 @@ mod test { assert_eq!(quota, back); }) } + + #[test] + fn test_to_quota() { + let quota = Quota { + max_burst: nonzero!(32_u32), + replenish_1_per: Duration::from_secs(3), + }; + let gcra = Gcra::new(quota); + let expect = gcra.to_quota(); + assert_eq!(quota, expect); + + let quota = Quota { + max_burst: nonzero!(94_u32), + replenish_1_per: Duration::from_millis(310), + }; + let gcra = Gcra::new(quota); + let expect = gcra.to_quota(); + assert_eq!(quota, expect); + + let quota = Quota { + max_burst: nonzero!(1016_u32), + replenish_1_per: Duration::from_micros(30310), + }; + let gcra = Gcra::new(quota); + let expect = gcra.to_quota(); + assert_eq!(quota, expect); + } } diff --git a/governor/src/state.rs b/governor/src/state.rs index 2ea0e7b3..d2f3868f 100644 --- a/governor/src/state.rs +++ b/governor/src/state.rs @@ -96,6 +96,11 @@ where pub fn into_state_store(self) -> S { self.state } + + /// Consumes the `RateLimiter` and returns `Quota`. + pub fn quota(&self) -> Quota { + self.gcra.to_quota() + } } impl RateLimiter diff --git a/governor/src/state/direct/future.rs b/governor/src/state/direct/future.rs index aee6d758..9ad686fa 100644 --- a/governor/src/state/direct/future.rs +++ b/governor/src/state/direct/future.rs @@ -106,7 +106,7 @@ mod test { #[test] fn insufficient_capacity_impl_coverage() { let i = InsufficientCapacity(1); - assert_eq!(i.0, i.clone().0); + assert_eq!(i.0, i.0); assert_gt!(format!("{}", i).len(), 0); } } From 9e05d0d4eef4b887d786d9a0f1d109a18a2844a2 Mon Sep 17 00:00:00 2001 From: koshell Date: Sun, 5 May 2024 13:57:27 +1000 Subject: [PATCH 2/6] better support non-unix development environments (+ some Cargo.toml formatting) --- governor/Cargo.toml | 16 ++++++++++++---- governor/tests/memory_leaks.rs | 1 + 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/governor/Cargo.toml b/governor/Cargo.toml index cf1686a9..972f5204 100644 --- a/governor/Cargo.toml +++ b/governor/Cargo.toml @@ -26,18 +26,26 @@ harness = false bench = false [dev-dependencies] -criterion = {version = "0.5.1", features = ["html_reports"]} +criterion = { version = "0.5.1", features = ["html_reports"] } tynm = "0.1.4" crossbeam = "0.8.0" -libc = "0.2.70" futures = "0.3.5" proptest = "1.0.0" all_asserts = "2.2.0" +[target.'cfg(unix)'.dev-dependencies] +libc = "0.2.70" + [features] default = ["std", "dashmap", "jitter", "quanta"] quanta = ["dep:quanta"] -std = ["no-std-compat/std", "nonzero_ext/std", "futures-timer", "futures", "dep:parking_lot"] +std = [ + "no-std-compat/std", + "nonzero_ext/std", + "futures-timer", + "futures", + "dep:parking_lot", +] jitter = ["rand"] no_std = ["no-std-compat/compat_hash"] @@ -51,7 +59,7 @@ futures = { version = "0.3.5", optional = true } rand = { version = "0.8.0", optional = true } dashmap = { version = "5.1.0", optional = true } quanta = { version = "0.12.0", optional = true } -no-std-compat = { version = "0.4.1", features = [ "alloc" ] } +no-std-compat = { version = "0.4.1", features = ["alloc"] } cfg-if = "1.0" # To ensure we don't pull in vulnerable smallvec, see https://github.com/antifuchs/governor/issues/60 diff --git a/governor/tests/memory_leaks.rs b/governor/tests/memory_leaks.rs index 4f6cfa33..caf639c2 100755 --- a/governor/tests/memory_leaks.rs +++ b/governor/tests/memory_leaks.rs @@ -1,4 +1,5 @@ #![cfg(feature = "std")] +#![cfg(unix)] // This test uses procinfo, so can only be run on Linux. extern crate libc; From 8bdeba5f51c8c4dc09ba494763ca67bab5dcb9b8 Mon Sep 17 00:00:00 2001 From: koshell Date: Sun, 5 May 2024 14:04:46 +1000 Subject: [PATCH 3/6] merging upstream changes with original PR --- .github/dependabot.yml | 36 ++++--- .github/workflows/benchmark.yml | 12 +++ .github/workflows/ci.yml | 54 ++++++++++ .github/workflows/ci_rust.yml | 23 +++++ .github/workflows/combine-prs.yml | 151 ---------------------------- .github/workflows/release_it.yml | 42 ++++++++ .github/workflows/release_pr.yml | 45 +++++++++ .github/workflows/tests.yml | 50 --------- LICENSE | 21 ++++ governor/CHANGELOG.md | 43 ++++++++ governor/Cargo.toml | 14 +-- governor/README.md | 2 +- governor/benches/multi_threaded.rs | 4 +- governor/benches/realtime_clock.rs | 4 +- governor/benches/single_threaded.rs | 4 +- governor/src/_guide.rs | 6 +- governor/src/clock.rs | 6 +- governor/src/clock/quanta.rs | 25 ++++- governor/src/gcra.rs | 6 +- governor/src/state.rs | 10 +- governor/src/state/direct.rs | 6 +- governor/src/state/in_memory.rs | 3 +- governor/src/state/keyed.rs | 10 +- governor/src/state/keyed/dashmap.rs | 4 +- governor/src/state/keyed/future.rs | 49 ++++++++- governor/src/state/keyed/hashmap.rs | 15 +-- governor/tests/direct.rs | 42 ++++++-- governor/tests/future.rs | 24 +++++ governor/tests/keyed_dashmap.rs | 10 +- governor/tests/keyed_hashmap.rs | 10 +- governor/tests/middleware.rs | 6 +- governor/tests/proptests.rs | 2 +- 32 files changed, 452 insertions(+), 287 deletions(-) create mode 100644 .github/workflows/benchmark.yml create mode 100644 .github/workflows/ci.yml create mode 100644 .github/workflows/ci_rust.yml delete mode 100644 .github/workflows/combine-prs.yml create mode 100644 .github/workflows/release_it.yml create mode 100644 .github/workflows/release_pr.yml delete mode 100644 .github/workflows/tests.yml create mode 100644 LICENSE diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 6c35b3cd..3709fd5e 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -1,14 +1,22 @@ -version: 2 -updates: -- package-ecosystem: cargo - directory: "/" - schedule: - interval: daily - time: "04:00" - open-pull-requests-limit: 10 -- package-ecosystem: github-actions - directory: "/" - schedule: - interval: daily - time: "04:00" - open-pull-requests-limit: 10 +"updates": +- "directory": "/" + "groups": + "cargo": + "patterns": + - "*" + "open-pull-requests-limit": 10 + "package-ecosystem": "cargo" + "schedule": + "interval": "daily" + "time": "04:00" +- "directory": "/" + "groups": + "github-actions": + "patterns": + - "*" + "open-pull-requests-limit": 10 + "package-ecosystem": "github-actions" + "schedule": + "interval": "daily" + "time": "04:00" +"version": 2 diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml new file mode 100644 index 00000000..493801cd --- /dev/null +++ b/.github/workflows/benchmark.yml @@ -0,0 +1,12 @@ +name: "CI/benchmark" +on: + workflow_call: + +jobs: + cargo_bench: + runs-on: ubuntu-latest + if: github.event_name == 'pull_request' + steps: + - uses: actions/checkout@v4.1.1 + - uses: dtolnay/rust-toolchain@stable + - run: "cargo bench" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 00000000..bd415247 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,54 @@ +# This file is automatically generated by terraform. I hope it's +# readable, but please don't edit it. + +"jobs": + "benchmark": + "secrets": "inherit" + "uses": "./.github/workflows/benchmark.yml" + "can_enqueue": + "if": "always() && github.event_name != 'merge_group'" + "needs": + - "ci_rust" + "permissions": + "actions": "read" + "runs-on": "ubuntu-latest" + "steps": + - "env": + "NEEDS_JSON": "${{toJSON(needs)}}" + "name": "Transform outcomes" + "run": | + echo "ALL_SUCCESS=$(echo "$NEEDS_JSON" | jq '. | to_entries | map([.value.result == "success", .value.result == "skipped"] | any) | all')" >>$GITHUB_ENV + - "name": "check" + "run": "[ $ALL_SUCCESS == true ]" + "can_merge": + "if": "always() && github.event_name == 'merge_group'" + "needs": + - "ci_rust" + "permissions": + "actions": "read" + "runs-on": "ubuntu-latest" + "steps": + - "env": + "NEEDS_JSON": "${{toJSON(needs)}}" + "name": "Transform outcomes" + "run": | + echo "ALL_SUCCESS=$(echo "$NEEDS_JSON" | jq '. | to_entries | map([.value.result == "success", .value.result == "skipped"] | any) | all')" >>$GITHUB_ENV + - "name": "check" + "run": "[ $ALL_SUCCESS == true ]" + "can_see_status": + "runs-on": "ubuntu-latest" + "steps": + - "name": "Immediate success for improved visibility on github merge queue" + "run": "true" + "ci_rust": + "secrets": "inherit" + "uses": "./.github/workflows/ci_rust.yml" +"name": "CI" +"on": + "merge_group": {} + "pull_request": + "branches": "master" + "push": + "branches": + - "master" + "workflow_dispatch": {} diff --git a/.github/workflows/ci_rust.yml b/.github/workflows/ci_rust.yml new file mode 100644 index 00000000..43829747 --- /dev/null +++ b/.github/workflows/ci_rust.yml @@ -0,0 +1,23 @@ +name: "CI/Rust" +on: + workflow_call: + +jobs: + rust_lints: + uses: "boinkor-net/ci-baseline-rust/.github/workflows/ci_baseline_rust_lints.yml@main" + with: + manifest_dir: . + apt_install_packages: "" + + rust_tests: + uses: "boinkor-net/ci-baseline-rust/.github/workflows/ci_baseline_rust_tests.yml@main" + strategy: + fail-fast: false + matrix: + rust_toolchain: ["nightly","stable"] + cargo_test_args: ["--no-default-features --features no_std","--no-default-features --features 'jitter no_std'","--no-default-features --features std",""] + with: + rust_toolchain: ${{matrix.rust_toolchain}} + cargo_test_args: ${{matrix.cargo_test_args}} + manifest_dir: . + apt_install_packages: "" diff --git a/.github/workflows/combine-prs.yml b/.github/workflows/combine-prs.yml deleted file mode 100644 index 2319fb57..00000000 --- a/.github/workflows/combine-prs.yml +++ /dev/null @@ -1,151 +0,0 @@ -name: "Combine PRs" - -# Controls when the action will run - in this case triggered manually -on: - workflow_dispatch: - inputs: - branchPrefix: - description: "Branch prefix to find combinable PRs based on" - required: true - default: "dependabot" - mustBeGreen: - description: "Only combine PRs that are green (status is success)" - required: true - default: true - combineBranchName: - description: "Name of the branch from which to submit the combined PR" - required: true - default: "combined-prs-branch" - ignoreLabel: - description: "Exclude PRs with this label" - required: true - default: "nocombine" - -# A workflow run is made up of one or more jobs that can run sequentially or in parallel -jobs: - # This workflow contains a single job called "combine-prs" - combine-prs: - # The type of runner that the job will run on - runs-on: ubuntu-latest - - # Steps represent a sequence of tasks that will be executed as part of the job - steps: - - uses: actions/github-script@v6 - id: create-combined-pr - name: Create Combined PR - with: - github-token: ${{secrets.GITHUB_TOKEN}} - script: | - const pulls = await github.paginate('GET /repos/:owner/:repo/pulls', { - owner: context.repo.owner, - repo: context.repo.repo - }); - let branchesAndPRStrings = []; - let baseBranch = null; - let baseBranchSHA = null; - for (const pull of pulls) { - const branch = pull['head']['ref']; - console.log('Pull for branch: ' + branch); - if (branch.startsWith('${{ github.event.inputs.branchPrefix }}')) { - console.log('Branch matched prefix: ' + branch); - let statusOK = true; - if(${{ github.event.inputs.mustBeGreen }}) { - console.log('Checking green status: ' + branch); - const stateQuery = `query($owner: String!, $repo: String!, $pull_number: Int!) { - repository(owner: $owner, name: $repo) { - pullRequest(number:$pull_number) { - commits(last: 1) { - nodes { - commit { - statusCheckRollup { - state - } - } - } - } - } - } - }` - const vars = { - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: pull['number'] - }; - const result = await github.graphql(stateQuery, vars); - const [{ commit }] = result.repository.pullRequest.commits.nodes; - const state = commit.statusCheckRollup.state - console.log('Validating status: ' + state); - if(state != 'SUCCESS') { - console.log('Discarding ' + branch + ' with status ' + state); - statusOK = false; - } - } - console.log('Checking labels: ' + branch); - const labels = pull['labels']; - for(const label of labels) { - const labelName = label['name']; - console.log('Checking label: ' + labelName); - if(labelName == '${{ github.event.inputs.ignoreLabel }}') { - console.log('Discarding ' + branch + ' with label ' + labelName); - statusOK = false; - } - } - if (statusOK) { - console.log('Adding branch to array: ' + branch); - const prString = '#' + pull['number'] + ' ' + pull['title']; - branchesAndPRStrings.push({ branch, prString }); - baseBranch = pull['base']['ref']; - baseBranchSHA = pull['base']['sha']; - } - } - } - if (branchesAndPRStrings.length == 0) { - core.setFailed('No PRs/branches matched criteria'); - return; - } - try { - await github.rest.git.createRef({ - owner: context.repo.owner, - repo: context.repo.repo, - ref: 'refs/heads/' + '${{ github.event.inputs.combineBranchName }}', - sha: baseBranchSHA - }); - } catch (error) { - console.log(error); - core.setFailed('Failed to create combined branch - maybe a branch by that name already exists?'); - return; - } - - let combinedPRs = []; - let mergeFailedPRs = []; - for(const { branch, prString } of branchesAndPRStrings) { - try { - await github.rest.repos.merge({ - owner: context.repo.owner, - repo: context.repo.repo, - base: '${{ github.event.inputs.combineBranchName }}', - head: branch, - }); - console.log('Merged branch ' + branch); - combinedPRs.push(prString); - } catch (error) { - console.log('Failed to merge branch ' + branch); - mergeFailedPRs.push(prString); - } - } - - console.log('Creating combined PR'); - const combinedPRsString = combinedPRs.join('\n'); - let body = '✅ This PR was created by the Combine PRs action by combining the following PRs:\n' + combinedPRsString; - if(mergeFailedPRs.length > 0) { - const mergeFailedPRsString = mergeFailedPRs.join('\n'); - body += '\n\n⚠️ The following PRs were left out due to merge conflicts:\n' + mergeFailedPRsString - } - await github.rest.pulls.create({ - owner: context.repo.owner, - repo: context.repo.repo, - title: 'Combined PR', - head: '${{ github.event.inputs.combineBranchName }}', - base: baseBranch, - body: body - }); diff --git a/.github/workflows/release_it.yml b/.github/workflows/release_it.yml new file mode 100644 index 00000000..207bd73e --- /dev/null +++ b/.github/workflows/release_it.yml @@ -0,0 +1,42 @@ +name: Cut a new release +on: + pull_request: + types: + - closed + +jobs: + tag_github_release: + if: github.event.pull_request.merged == true && contains(github.event.pull_request.labels.*.name, 'release') + + permissions: + id-token: write # Enable OIDC + contents: write + + runs-on: ubuntu-latest + steps: + - run: echo merge commit is $GITHUB_SHA, from PR branch ${{ github.event.pull_request.head.ref }} + - uses: actions-ecosystem/action-regex-match@v2 + id: version + with: + text: ${{ github.event.pull_request.head.ref }} + regex: "^release/([^/]+)/(.+)$" + # Name of crate is steps.version.outputs.group1, version is steps.version.outputs.group2. + # TODO: figure out how this works with workspaces & multiple crates + + - name: Create release v${{steps.version.outputs.group2}} + uses: softprops/action-gh-release@v1 + with: + tag_name: v${{ steps.version.outputs.group2 }} + name: ${{steps.version.outputs.group1}} v${{ steps.version.outputs.group2 }} + target_commitish: ${{github.sha}} + + - name: Install stable toolchain + uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: stable + + - uses: actions/checkout@v4 + - run: cargo publish --token ${CRATES_TOKEN} -p ${{ steps.version.outputs.group1 }} + env: + CRATES_TOKEN: ${{ secrets.CRATESIO_RELEASE_TOKEN }} diff --git a/.github/workflows/release_pr.yml b/.github/workflows/release_pr.yml new file mode 100644 index 00000000..90aa09ed --- /dev/null +++ b/.github/workflows/release_pr.yml @@ -0,0 +1,45 @@ +name: Open a release PR +on: + workflow_dispatch: + inputs: + version: + description: Version to release + required: true + type: string + +jobs: + make-release-pr: + permissions: + id-token: write # Enable OIDC + pull-requests: write + contents: write + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: chainguard-dev/actions/setup-gitsign@main + - name: Install cargo-release + uses: taiki-e/cache-cargo-install-action@v1 + with: + tool: cargo-release + - name: Install cargo-semver-checks + uses: taiki-e/cache-cargo-install-action@v1 + with: + tool: cargo-semver-checks + + - uses: cargo-bins/release-pr@v2 + id: pr + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + version: ${{ inputs.version }} + check-semver: true + crate-release-all: true # all in workspace + - uses: actions-ecosystem/action-regex-match@v2 + id: pr-id + with: + text: ${{ steps.pr.outputs.pr-url }} + regex: "/pull/([0-9]+)$" + - uses: actions-ecosystem/action-add-labels@v1 + with: + github_token: ${{ secrets.GITHUB_TOKEN }} + labels: release + number: ${{ steps.pr-id.outputs.group1 }} diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml deleted file mode 100644 index f68efd95..00000000 --- a/.github/workflows/tests.yml +++ /dev/null @@ -1,50 +0,0 @@ -name: "CI" -# This workflow is triggered on all pushes to the repository, or on Friday night. -on: - pull_request: {} - push: - branches: - - master - - staging - - trying - schedule: - - cron: "0 23 * * FRI" - workflow_call: - -jobs: - rust_lints: - secrets: "inherit" - uses: "boinkor-net/ci-baseline-rust/.github/workflows/ci_baseline_rust_lints.yml@main" - - rust_tests: - secrets: "inherit" - uses: "boinkor-net/ci-baseline-rust/.github/workflows/ci_baseline_rust_tests.yml@main" - strategy: - matrix: - rust_toolchain: [nightly, stable] - cargo_test_args: - - "--no-default-features --features no_std" - - "--no-default-features --features 'jitter no_std'" - - "--no-default-features --features std" - - "" - with: - rust_toolchain: ${{matrix.rust_toolchain}} - cargo_test_args: ${{matrix.cargo_test_args}} - - cargo_bench: - runs-on: ubuntu-latest - if: github.event_name == 'pull_request' - needs: rust_tests - steps: - - uses: actions/checkout@v3.5.2 - - uses: dtolnay/rust-toolchain@stable - - run: "cargo bench" - - bors_passing: - runs-on: ubuntu-latest - needs: - - rust_tests - - rust_lints - steps: - - run: true - name: success diff --git a/LICENSE b/LICENSE new file mode 100644 index 00000000..51c6a41e --- /dev/null +++ b/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2023 Andreas Fuchs + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/governor/CHANGELOG.md b/governor/CHANGELOG.md index 51cf8df2..243d572f 100644 --- a/governor/CHANGELOG.md +++ b/governor/CHANGELOG.md @@ -4,6 +4,49 @@ ## [Unreleased] - ReleaseDate +## [[0.6.3](https://docs.rs/governor/0.6.3/governor/)] - 2024-02-16 + +### Changed + +* Just another bug-fixed release process. This time, it should + actually release out of github actions. + +## [[0.6.2](https://docs.rs/governor/0.6.2/governor/)] - 2024-02-16 + +### Changed + +* New release process: governor can now be automatically released + using github actions. + +## [[0.6.1](https://docs.rs/governor/0.6.1/governor/)] - 2024-02-16 + +### Changed + +* The governor repo now lives in the `boinkor-net` github + organization. No ownership has changed (@antifuchs still manages + this org), but this makes it easier to securely manage the CI and + release setup. + +* The `.per_second` constructor for `Quota` now constructs a quota + that ensures all rate-limiting calls succeed when given values in + excess of 1 billion (previously, this would result in rate limiters + that would incorrectly reject values). Reported in + [#203](https://github.com/antifuchs/governor/issues/203). + +* `QuantaUpkeepInstant` now [properly advances + forward](https://github.com/boinkor-net/governor/pull/223). + +* `no_std` is now [properly + supported](https://github.com/boinkor-net/governor/pull/222): + Instead of parking-lot, governor now uses the spinning_top crate in + `no_std` mode. + +### Contributors +* [@rkd-msw](https://github.com/rkd-msw) +* [@Serene-Arc](https://github.com/Serene-Arc) +* [@waynerobinson](https://github.com/waynerobinson) +* [@mammothbane](https://github.com/mammothbane) + ## [[0.6.0](https://docs.rs/governor/0.6.0/governor/)] - 2023-07-12 ### Added diff --git a/governor/Cargo.toml b/governor/Cargo.toml index 4b168bbf..cf1686a9 100644 --- a/governor/Cargo.toml +++ b/governor/Cargo.toml @@ -1,11 +1,11 @@ [package] name = "governor" -version = "0.6.0" +version = "0.6.3" authors = ["Andreas Fuchs "] edition = "2018" license = "MIT" -homepage = "https://github.com/antifuchs/governor" -repository = "https://github.com/antifuchs/governor.git" +homepage = "https://github.com/boinkor-net/governor" +repository = "https://github.com/boinkor-net/governor.git" readme = "README.md" description = "A rate-limiting implementation in Rust" documentation = "https://docs.rs/governor" @@ -37,18 +37,20 @@ all_asserts = "2.2.0" [features] default = ["std", "dashmap", "jitter", "quanta"] quanta = ["dep:quanta"] -std = ["no-std-compat/std", "nonzero_ext/std", "futures-timer", "futures"] +std = ["no-std-compat/std", "nonzero_ext/std", "futures-timer", "futures", "dep:parking_lot"] jitter = ["rand"] no_std = ["no-std-compat/compat_hash"] [dependencies] nonzero_ext = { version = "0.3.0", default-features = false } -parking_lot = "0.12.0" +parking_lot = { version = "0.12", optional = true } +spinning_top = "0.3" +portable-atomic = { version = "1.6", features = ["require-cas"] } futures-timer = { version = "3.0.2", optional = true } futures = { version = "0.3.5", optional = true } rand = { version = "0.8.0", optional = true } dashmap = { version = "5.1.0", optional = true } -quanta = { version = "0.11.1", optional = true } +quanta = { version = "0.12.0", optional = true } no-std-compat = { version = "0.4.1", features = [ "alloc" ] } cfg-if = "1.0" diff --git a/governor/README.md b/governor/README.md index c5966d1a..56ca55a9 100644 --- a/governor/README.md +++ b/governor/README.md @@ -1,4 +1,4 @@ -![Build status](https://github.com/antifuchs/governor/actions/workflows/ci_push.yml/badge.svg?branch=master) [![codecov](https://codecov.io/gh/antifuchs/governor/branch/master/graph/badge.svg)](https://codecov.io/gh/antifuchs/governor) [![Docs](https://docs.rs/governor/badge.svg)](https://docs.rs/governor/) [![crates.io](https://img.shields.io/crates/v/governor.svg)](https://crates.io/crates/governor) +![Build status](https://github.com/boinkor-net/governor/actions/workflows/ci_push.yml/badge.svg?branch=master) [![codecov](https://codecov.io/gh/boinkor-net/governor/branch/master/graph/badge.svg)](https://codecov.io/gh/boinkor-net/governor) [![Docs](https://docs.rs/governor/badge.svg)](https://docs.rs/governor/) [![crates.io](https://img.shields.io/crates/v/governor.svg)](https://crates.io/crates/governor) # governor - a library for regulating the flow of data diff --git a/governor/benches/multi_threaded.rs b/governor/benches/multi_threaded.rs index 2a864ca9..64c5a086 100644 --- a/governor/benches/multi_threaded.rs +++ b/governor/benches/multi_threaded.rs @@ -32,7 +32,7 @@ fn bench_direct(c: &mut Criterion) { b.iter_custom(|iters| { let lim = Arc::new(RateLimiter::direct_with_clock( Quota::per_second(nonzero!(50u32)), - &clock, + clock.clone(), )); let mut children = vec![]; let start = Instant::now(); @@ -68,7 +68,7 @@ fn bench_keyed + Default + Send + Sync + 'static>(c: &mu let lim: Arc> = Arc::new(RateLimiter::new( Quota::per_second(nonzero!(50u32)), state, - &clock, + clock.clone(), )); let mut children = vec![]; diff --git a/governor/benches/realtime_clock.rs b/governor/benches/realtime_clock.rs index 864d2ad3..0c59c85c 100644 --- a/governor/benches/realtime_clock.rs +++ b/governor/benches/realtime_clock.rs @@ -38,7 +38,7 @@ fn bench_mostly_allow(c: &mut Criterion) { with_realtime_clocks! {("mostly_allow", group) |b, clock| { let rl = RateLimiter::direct_with_clock( #[allow(deprecated)] Quota::new(nonzero!(u32::max_value()), Duration::from_nanos(1)).unwrap(), - clock + clock.clone() ); b.iter(|| { black_box(rl.check().is_ok()); @@ -51,7 +51,7 @@ fn bench_mostly_deny(c: &mut Criterion) { let mut group = c.benchmark_group("realtime_clock"); group.throughput(Throughput::Elements(1)); with_realtime_clocks! {("mostly_deny", group) |b, clock| { - let rl = RateLimiter::direct_with_clock(Quota::per_hour(nonzero!(1u32)), clock); + let rl = RateLimiter::direct_with_clock(Quota::per_hour(nonzero!(1u32)), clock.clone()); b.iter(|| { black_box(rl.check().is_ok()); }); diff --git a/governor/benches/single_threaded.rs b/governor/benches/single_threaded.rs index bf496881..b9caa5f1 100644 --- a/governor/benches/single_threaded.rs +++ b/governor/benches/single_threaded.rs @@ -20,7 +20,7 @@ fn bench_direct(c: &mut Criterion) { group.bench_function("direct", |b| { let clock = clock::FakeRelativeClock::default(); let step = Duration::from_millis(20); - let rl = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(50u32)), &clock); + let rl = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(50u32)), clock.clone()); b.iter_batched( || { clock.advance(step); @@ -48,7 +48,7 @@ fn bench_keyed + Default + Send + Sync + 'static>(c: &mu _, _, NoOpMiddleware<::Instant>, - > = RateLimiter::new(Quota::per_second(nonzero!(50u32)), state, &clock); + > = RateLimiter::new(Quota::per_second(nonzero!(50u32)), state, clock.clone()); b.iter_batched( || { clock.advance(step); diff --git a/governor/src/_guide.rs b/governor/src/_guide.rs index 8fb20e06..a6ff3fcb 100644 --- a/governor/src/_guide.rs +++ b/governor/src/_guide.rs @@ -62,7 +62,7 @@ //! # use nonzero_ext::*; //! # use governor::{clock::FakeRelativeClock, RateLimiter, Quota}; //! let clock = FakeRelativeClock::default(); -//! RateLimiter::direct_with_clock(Quota::per_second(nonzero!(50u32)), &clock); +//! RateLimiter::direct_with_clock(Quota::per_second(nonzero!(50u32)), clock); //! ``` //! //! #### Constructing a keyed rate limiter @@ -92,7 +92,7 @@ //! function arguments. The [`crate::RateLimiter`] type signatures //! tend to be pretty unwieldy for that, so this crate exports a pair //! of handy type aliases, [`crate::DefaultDirectRateLimiter`] and -//! [`crate::DefaultDirectRateLimiter`]. +//! [`crate::DefaultKeyedRateLimiter`]. //! //! Here's an example for embedding a direct rate limiter in a struct: //! @@ -144,7 +144,7 @@ //! # use std::time::Duration; //! //! let mut clock = FakeRelativeClock::default(); -//! let lim = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(20u32)), &clock); +//! let lim = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(20u32)), clock); //! let ms = Duration::from_millis(1); //! //! crossbeam::scope(|scope| { diff --git a/governor/src/clock.rs b/governor/src/clock.rs index 9971a6d9..7895bcdd 100644 --- a/governor/src/clock.rs +++ b/governor/src/clock.rs @@ -33,7 +33,6 @@ //! } //! } //! -//! #[derive(Clone)] //! struct MyCounter(u64); //! //! impl Clock for MyCounter { @@ -50,11 +49,12 @@ use std::prelude::v1::*; use std::convert::TryInto; use std::fmt::Debug; use std::ops::Add; -use std::sync::atomic::AtomicU64; use std::sync::atomic::Ordering; use std::sync::Arc; use std::time::Duration; +use portable_atomic::AtomicU64; + use crate::nanos::Nanos; /// A measurement from a clock. @@ -74,7 +74,7 @@ pub trait Reference: } /// A time source used by rate limiters. -pub trait Clock: Clone { +pub trait Clock { /// A measurement of a monotonically increasing clock. type Instant: Reference; diff --git a/governor/src/clock/quanta.rs b/governor/src/clock/quanta.rs index 4a0b571f..12383d31 100644 --- a/governor/src/clock/quanta.rs +++ b/governor/src/clock/quanta.rs @@ -100,8 +100,9 @@ impl Clock for QuantaUpkeepClock { fn now(&self) -> Self::Instant { QuantaInstant(Nanos::from( - self.reference - .saturating_duration_since(self.clock.recent()), + self.clock + .recent() + .saturating_duration_since(self.reference), )) } } @@ -116,6 +117,7 @@ mod test { use super::*; use crate::clock::{Clock, QuantaClock, QuantaUpkeepClock, Reference}; use crate::nanos::Nanos; + use std::thread; use std::time::Duration; #[test] @@ -133,11 +135,11 @@ mod test { } #[test] - fn quanta_upkeep_impls_coverage() { + fn quanta_upkeep_impls_coverage_and_advances() { let one_ns = Nanos::new(1); // let _c1 = // QuantaUpkeepClock::from_builder(quanta::Upkeep::new(Duration::from_secs(1))).unwrap(); - let c = QuantaUpkeepClock::from_interval(Duration::from_secs(1)).unwrap(); + let c = QuantaUpkeepClock::from_interval(Duration::from_millis(50)).unwrap(); let now = c.now(); assert_ne!(now + one_ns, now); assert_eq!(one_ns, Reference::duration_since(&(now + one_ns), now)); @@ -146,5 +148,20 @@ mod test { Reference::saturating_sub(&(now + Duration::from_nanos(1).into()), one_ns), now ); + + // Test clock advances over time. + // (included in one test as only one QuantaUpkeepClock thread can be run at a time) + let start = c.now(); + for i in 0..5 { + thread::sleep(Duration::from_millis(250)); + let now = c.now(); + assert!( + now > start, + "now={:?} not after start={:?} on iteration={}", + now, + start, + i + ); + } } } diff --git a/governor/src/gcra.rs b/governor/src/gcra.rs index 7e698ff6..da8bf8d5 100644 --- a/governor/src/gcra.rs +++ b/governor/src/gcra.rs @@ -85,7 +85,9 @@ pub(crate) struct Gcra { impl Gcra { pub(crate) fn new(quota: Quota) -> Self { - let tau: Nanos = (quota.replenish_1_per * quota.max_burst.get()).into(); + let tau: Nanos = (cmp::max(quota.replenish_1_per, Duration::from_nanos(1)) + * quota.max_burst.get()) + .into(); let t: Nanos = quota.replenish_1_per.into(); Gcra { t, tau } } @@ -217,7 +219,7 @@ mod test { let clock = FakeRelativeClock::default(); let quota = Quota::per_second(nonzero!(1u32)); - let lb = RateLimiter::direct_with_clock(quota, &clock); + let lb = RateLimiter::direct_with_clock(quota, clock); assert!(lb.check().is_ok()); assert!(lb .check() diff --git a/governor/src/state.rs b/governor/src/state.rs index d2f3868f..b5b850e5 100644 --- a/governor/src/state.rs +++ b/governor/src/state.rs @@ -24,7 +24,7 @@ pub use direct::*; /// do more than N tasks a day). The keyed kind allows one rate limit per key (e.g. an API /// call budget per client API key). /// -/// A direct state store is expressed as [`StateStore::Key`] = [`NotKeyed`][direct::NotKeyed]. +/// A direct state store is expressed as [`StateStore::Key`] = [`NotKeyed`]. /// Keyed state stores have a /// type parameter for the key and set their key to that. pub trait StateStore { @@ -77,10 +77,9 @@ where /// /// This is the most generic way to construct a rate-limiter; most users should prefer /// [`direct`] or other methods instead. - pub fn new(quota: Quota, state: S, clock: &C) -> Self { + pub fn new(quota: Quota, state: S, clock: C) -> Self { let gcra = Gcra::new(quota); let start = clock.now(); - let clock = clock.clone(); RateLimiter { state, clock, @@ -97,6 +96,11 @@ where self.state } + /// Returns a reference to the clock. + pub fn clock(&self) -> &C { + &self.clock + } + /// Consumes the `RateLimiter` and returns `Quota`. pub fn quota(&self) -> Quota { self.gcra.to_quota() diff --git a/governor/src/state/direct.rs b/governor/src/state/direct.rs index 9310a6c8..c35aa336 100644 --- a/governor/src/state/direct.rs +++ b/governor/src/state/direct.rs @@ -46,7 +46,7 @@ impl RateLimiter { quota: Quota, ) -> RateLimiter { let clock = clock::DefaultClock::default(); - Self::direct_with_clock(quota, &clock) + Self::direct_with_clock(quota, clock) } } @@ -55,7 +55,7 @@ where C: clock::Clock, { /// Constructs a new direct rate limiter for a quota with a custom clock. - pub fn direct_with_clock(quota: Quota, clock: &C) -> Self { + pub fn direct_with_clock(quota: Quota, clock: C) -> Self { let state: InMemoryState = Default::default(); RateLimiter::new(quota, state, clock) } @@ -112,8 +112,6 @@ where #[cfg(feature = "std")] mod future; -#[cfg(feature = "std")] -pub use future::*; #[cfg(feature = "std")] mod sinks; diff --git a/governor/src/state/in_memory.rs b/governor/src/state/in_memory.rs index 100b0569..8e0a7e7e 100644 --- a/governor/src/state/in_memory.rs +++ b/governor/src/state/in_memory.rs @@ -5,10 +5,11 @@ use crate::state::{NotKeyed, StateStore}; use std::fmt; use std::fmt::Debug; use std::num::NonZeroU64; -use std::sync::atomic::AtomicU64; use std::sync::atomic::Ordering; use std::time::Duration; +use portable_atomic::AtomicU64; + /// An in-memory representation of a GCRA's rate-limiting state. /// /// Implemented using [`AtomicU64`] operations, this state representation can be used to diff --git a/governor/src/state/keyed.rs b/governor/src/state/keyed.rs index 1ff92e77..3203bcb4 100644 --- a/governor/src/state/keyed.rs +++ b/governor/src/state/keyed.rs @@ -43,7 +43,7 @@ where pub fn keyed(quota: Quota) -> Self { let state = DefaultKeyedStateStore::default(); let clock = clock::DefaultClock::default(); - RateLimiter::new(quota, state, &clock) + RateLimiter::new(quota, state, clock) } #[cfg(all(feature = "std", feature = "dashmap"))] @@ -51,7 +51,7 @@ where pub fn dashmap(quota: Quota) -> Self { let state = DashMapStateStore::default(); let clock = clock::DefaultClock::default(); - RateLimiter::new(quota, state, &clock) + RateLimiter::new(quota, state, clock) } #[cfg(any(all(feature = "std", not(feature = "dashmap")), not(feature = "std")))] @@ -60,7 +60,7 @@ where pub fn hashmap(quota: Quota) -> Self { let state = HashMapStateStore::default(); let clock = clock::DefaultClock::default(); - RateLimiter::new(quota, state, &clock) + RateLimiter::new(quota, state, clock) } } @@ -74,7 +74,7 @@ where pub fn hashmap(quota: Quota) -> Self { let state = HashMapStateStore::default(); let clock = clock::DefaultClock::default(); - RateLimiter::new(quota, state, &clock) + RateLimiter::new(quota, state, clock) } } @@ -281,7 +281,7 @@ mod test { > = RateLimiter::new( Quota::per_second(nonzero!(1_u32)), NaiveKeyedStateStore::default(), - &FakeRelativeClock::default(), + FakeRelativeClock::default(), ); assert_eq!(lim.check_key(&1u32), Ok(())); assert!(lim.is_empty()); diff --git a/governor/src/state/keyed/dashmap.rs b/governor/src/state/keyed/dashmap.rs index 43535110..5f67ccd1 100755 --- a/governor/src/state/keyed/dashmap.rs +++ b/governor/src/state/keyed/dashmap.rs @@ -36,8 +36,8 @@ where C: clock::Clock, { /// Constructs a new rate limiter with a custom clock, backed by a - /// [`DashMap`][dashmap::DashMap]. - pub fn dashmap_with_clock(quota: Quota, clock: &C) -> Self { + /// [`DashMap`]. + pub fn dashmap_with_clock(quota: Quota, clock: C) -> Self { let state: DashMapStateStore = DashMap::default(); RateLimiter::new(quota, state, clock) } diff --git a/governor/src/state/keyed/future.rs b/governor/src/state/keyed/future.rs index 84ca757f..c271faca 100644 --- a/governor/src/state/keyed/future.rs +++ b/governor/src/state/keyed/future.rs @@ -1,11 +1,11 @@ use std::prelude::v1::*; use crate::{ - clock, middleware::RateLimitingMiddleware, state::keyed::KeyedStateStore, Jitter, NotUntil, - RateLimiter, + clock, errors::InsufficientCapacity, middleware::RateLimitingMiddleware, + state::keyed::KeyedStateStore, Jitter, NotUntil, RateLimiter, }; use futures_timer::Delay; -use std::hash::Hash; +use std::{hash::Hash, num::NonZeroU32}; #[cfg(feature = "std")] /// # Keyed rate limiters - `async`/`await` @@ -57,4 +57,47 @@ where } } } + + /// Asynchronously resolves as soon as the rate limiter allows it. + /// + /// This is similar to `until_key_ready` except it waits for an abitrary number + /// of `n` cells to be available. + /// + /// Returns `InsufficientCapacity` if the `n` provided exceeds the maximum + /// capacity of the rate limiter. + pub async fn until_key_n_ready( + &self, + key: &K, + n: NonZeroU32, + ) -> Result { + self.until_key_n_ready_with_jitter(key, n, Jitter::NONE) + .await + } + + /// Asynchronously resolves as soon as the rate limiter allows it, with a + /// randomized wait period. + /// + /// This is similar to `until_key_ready_with_jitter` except it waits for an + /// abitrary number of `n` cells to be available. + /// + /// Returns `InsufficientCapacity` if the `n` provided exceeds the maximum + /// capacity of the rate limiter. + pub async fn until_key_n_ready_with_jitter( + &self, + key: &K, + n: NonZeroU32, + jitter: Jitter, + ) -> Result { + loop { + match self.check_key_n(key, n)? { + Ok(x) => { + return Ok(x); + } + Err(negative) => { + let delay = Delay::new(jitter + negative.wait_time_from(self.clock.now())); + delay.await; + } + } + } + } } diff --git a/governor/src/state/keyed/hashmap.rs b/governor/src/state/keyed/hashmap.rs index ca34e128..891bc481 100644 --- a/governor/src/state/keyed/hashmap.rs +++ b/governor/src/state/keyed/hashmap.rs @@ -10,7 +10,12 @@ use std::collections::HashMap; use std::hash::Hash; use crate::state::keyed::ShrinkableKeyedStateStore; -use parking_lot::Mutex; + +#[cfg(feature = "std")] +type Mutex = parking_lot::Mutex; + +#[cfg(not(feature = "std"))] +type Mutex = spinning_top::Spinlock; /// A thread-safe (but not very performant) implementation of a keyed rate limiter state /// store using [`HashMap`]. @@ -32,9 +37,7 @@ impl StateStore for HashMapStateStore { return v.measure_and_replace_one(f); } // not-so-fast path: make a new entry and measure it. - let entry = (*map) - .entry(key.clone()) - .or_insert_with(InMemoryState::default); + let entry = (*map).entry(key.clone()).or_default(); entry.measure_and_replace_one(f) } } @@ -67,8 +70,8 @@ where C: clock::Clock, { /// Constructs a new rate limiter with a custom clock, backed by a [`HashMap`]. - pub fn hashmap_with_clock(quota: Quota, clock: &C) -> Self { - let state: HashMapStateStore = Mutex::new(HashMap::new()); + pub fn hashmap_with_clock(quota: Quota, clock: C) -> Self { + let state: HashMapStateStore = HashMapStateStore::new(HashMap::new()); RateLimiter::new(quota, state, clock) } } diff --git a/governor/tests/direct.rs b/governor/tests/direct.rs index 95602bc2..62562923 100644 --- a/governor/tests/direct.rs +++ b/governor/tests/direct.rs @@ -8,14 +8,14 @@ use std::time::Duration; #[test] fn accepts_first_cell() { let clock = FakeRelativeClock::default(); - let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(5u32)), &clock); + let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(5u32)), clock); assert_eq!(Ok(()), lb.check()); } #[test] fn rejects_too_many() { let clock = FakeRelativeClock::default(); - let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(2u32)), &clock); + let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(2u32)), clock.clone()); let ms = Duration::from_millis(1); // use up our burst capacity (2 in the first second): @@ -39,7 +39,7 @@ fn rejects_too_many() { #[test] fn all_1_identical_to_1() { let clock = FakeRelativeClock::default(); - let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(2u32)), &clock); + let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(2u32)), clock.clone()); let ms = Duration::from_millis(1); let one = nonzero!(1u32); @@ -64,7 +64,7 @@ fn all_1_identical_to_1() { #[test] fn never_allows_more_than_capacity_all() { let clock = FakeRelativeClock::default(); - let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(4u32)), &clock); + let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(4u32)), clock.clone()); let ms = Duration::from_millis(1); // Use up the burst capacity: @@ -87,7 +87,7 @@ fn never_allows_more_than_capacity_all() { #[test] fn rejects_too_many_all() { let clock = FakeRelativeClock::default(); - let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(5u32)), &clock); + let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(5u32)), clock.clone()); let ms = Duration::from_millis(1); // Should not allow the first 15 cells on a capacity 5 bucket: @@ -101,7 +101,7 @@ fn rejects_too_many_all() { #[test] fn all_capacity_check_rejects_excess() { let clock = FakeRelativeClock::default(); - let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(5u32)), &clock); + let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(5u32)), clock); assert_eq!(Err(InsufficientCapacity(5)), lb.check_n(nonzero!(15u32))); assert_eq!(Err(InsufficientCapacity(5)), lb.check_n(nonzero!(6u32))); @@ -112,7 +112,7 @@ fn all_capacity_check_rejects_excess() { fn correct_wait_time() { let clock = FakeRelativeClock::default(); // Bucket adding a new element per 200ms: - let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(5u32)), &clock); + let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(5u32)), clock.clone()); let ms = Duration::from_millis(1); let mut conforming = 0; for _i in 0..20 { @@ -137,7 +137,7 @@ fn actual_threadsafety() { use crossbeam; let clock = FakeRelativeClock::default(); - let lim = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(20u32)), &clock); + let lim = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(20u32)), clock.clone()); let ms = Duration::from_millis(1); crossbeam::scope(|scope| { @@ -159,6 +159,30 @@ fn actual_threadsafety() { fn default_direct() { let clock = governor::clock::DefaultClock::default(); let limiter: DefaultDirectRateLimiter = - RateLimiter::direct_with_clock(Quota::per_second(nonzero!(20u32)), &clock); + RateLimiter::direct_with_clock(Quota::per_second(nonzero!(20u32)), clock); assert_eq!(Ok(()), limiter.check()); } + +#[cfg(feature = "std")] +#[test] +fn stresstest_large_quotas() { + use std::{sync::Arc, thread}; + + use governor::middleware::StateInformationMiddleware; + + let quota = Quota::per_second(nonzero!(1_000_000_001u32)); + let rate_limiter = + Arc::new(RateLimiter::direct(quota).with_middleware::()); + + fn rlspin(rl: Arc>) { + for _ in 0..1_000_000 { + rl.check().map_err(|e| dbg!(e)).unwrap(); + } + } + + let rate_limiter2 = rate_limiter.clone(); + thread::spawn(move || { + rlspin(rate_limiter2); + }); + rlspin(rate_limiter); +} diff --git a/governor/tests/future.rs b/governor/tests/future.rs index bf20f6e6..c4145ab9 100755 --- a/governor/tests/future.rs +++ b/governor/tests/future.rs @@ -55,6 +55,18 @@ fn pauses_keyed() { assert_ge!(i.elapsed(), Duration::from_millis(100)); } +#[test] +fn pauses_keyed_n() { + let lim = RateLimiter::keyed(Quota::per_second(nonzero!(10u32))); + + for _ in 0..6 { + lim.check_key(&1u32).unwrap(); + } + let i = Instant::now(); + block_on(lim.until_key_n_ready(&1u32, nonzero!(5u32))).unwrap(); + assert_ge!(i.elapsed(), Duration::from_millis(100)); +} + #[test] fn proceeds() { let lim = RateLimiter::direct(Quota::per_second(nonzero!(2u32))); @@ -79,6 +91,14 @@ fn proceeds_keyed() { assert_le!(i.elapsed(), MAX_TEST_RUN_DURATION); } +#[test] +fn proceeds_keyed_n() { + let lim = RateLimiter::keyed(Quota::per_second(nonzero!(3u32))); + let i = Instant::now(); + block_on(lim.until_key_n_ready(&1u32, nonzero!(2u32))).unwrap(); + assert_le!(i.elapsed(), MAX_TEST_RUN_DURATION); +} + #[test] fn multiple() { let lim = Arc::new(RateLimiter::direct(Quota::per_second(nonzero!(10u32)))); @@ -125,4 +145,8 @@ fn errors_on_exceeded_capacity() { let lim = RateLimiter::direct(Quota::per_second(nonzero!(10u32))); block_on(lim.until_n_ready(nonzero!(11u32))).unwrap_err(); + + let lim = RateLimiter::keyed(Quota::per_second(nonzero!(10u32))); + + block_on(lim.until_key_n_ready(&1u32, nonzero!(11u32))).unwrap_err(); } diff --git a/governor/tests/keyed_dashmap.rs b/governor/tests/keyed_dashmap.rs index 2c427f31..cd1bb599 100755 --- a/governor/tests/keyed_dashmap.rs +++ b/governor/tests/keyed_dashmap.rs @@ -14,7 +14,7 @@ const KEYS: &[u32] = &[1u32, 2u32]; #[test] fn accepts_first_cell() { let clock = FakeRelativeClock::default(); - let lb = RateLimiter::dashmap_with_clock(Quota::per_second(nonzero!(5u32)), &clock); + let lb = RateLimiter::dashmap_with_clock(Quota::per_second(nonzero!(5u32)), clock.clone()); for key in KEYS { assert_eq!(Ok(()), lb.check_key(&key), "key {:?}", key); } @@ -23,7 +23,7 @@ fn accepts_first_cell() { #[test] fn rejects_too_many() { let clock = FakeRelativeClock::default(); - let lb = RateLimiter::dashmap_with_clock(Quota::per_second(nonzero!(2u32)), &clock); + let lb = RateLimiter::dashmap_with_clock(Quota::per_second(nonzero!(2u32)), clock.clone()); let ms = Duration::from_millis(1); for key in KEYS { @@ -71,7 +71,7 @@ fn expiration() { let ms = Duration::from_millis(1); let make_bucket = || { - let lim = RateLimiter::dashmap_with_clock(Quota::per_second(nonzero!(1u32)), &clock); + let lim = RateLimiter::dashmap_with_clock(Quota::per_second(nonzero!(1u32)), clock.clone()); lim.check_key(&"foo").unwrap(); clock.advance(ms * 200); lim.check_key(&"bar").unwrap(); @@ -107,7 +107,7 @@ fn actual_threadsafety() { use crossbeam; let clock = FakeRelativeClock::default(); - let lim = RateLimiter::dashmap_with_clock(Quota::per_second(nonzero!(20u32)), &clock); + let lim = RateLimiter::dashmap_with_clock(Quota::per_second(nonzero!(20u32)), clock.clone()); let ms = Duration::from_millis(1); for key in KEYS { @@ -150,7 +150,7 @@ fn dashmap_length() { fn dashmap_shrink_to_fit() { let clock = FakeRelativeClock::default(); // a steady rate of 3ms between elements: - let lim = RateLimiter::dashmap_with_clock(Quota::per_second(nonzero!(20u32)), &clock); + let lim = RateLimiter::dashmap_with_clock(Quota::per_second(nonzero!(20u32)), clock.clone()); let ms = Duration::from_millis(1); assert_eq!( diff --git a/governor/tests/keyed_hashmap.rs b/governor/tests/keyed_hashmap.rs index f7eafd02..8663ac89 100644 --- a/governor/tests/keyed_hashmap.rs +++ b/governor/tests/keyed_hashmap.rs @@ -12,7 +12,7 @@ const KEYS: &[u32] = &[1u32, 2u32]; #[test] fn accepts_first_cell() { let clock = FakeRelativeClock::default(); - let lb = RateLimiter::hashmap_with_clock(Quota::per_second(nonzero!(5u32)), &clock); + let lb = RateLimiter::hashmap_with_clock(Quota::per_second(nonzero!(5u32)), clock.clone()); for key in KEYS { assert_eq!(Ok(()), lb.check_key(&key), "key {:?}", key); } @@ -21,7 +21,7 @@ fn accepts_first_cell() { #[test] fn rejects_too_many() { let clock = FakeRelativeClock::default(); - let lb = RateLimiter::hashmap_with_clock(Quota::per_second(nonzero!(2u32)), &clock); + let lb = RateLimiter::hashmap_with_clock(Quota::per_second(nonzero!(2u32)), clock.clone()); let ms = Duration::from_millis(1); for key in KEYS { @@ -65,7 +65,7 @@ fn expiration() { let ms = Duration::from_millis(1); let make_bucket = || { - let lim = RateLimiter::hashmap_with_clock(Quota::per_second(nonzero!(1u32)), &clock); + let lim = RateLimiter::hashmap_with_clock(Quota::per_second(nonzero!(1u32)), clock.clone()); lim.check_key(&"foo").unwrap(); clock.advance(ms * 200); lim.check_key(&"bar").unwrap(); @@ -101,7 +101,7 @@ fn actual_threadsafety() { use crossbeam; let clock = FakeRelativeClock::default(); - let lim = RateLimiter::hashmap_with_clock(Quota::per_second(nonzero!(20u32)), &clock); + let lim = RateLimiter::hashmap_with_clock(Quota::per_second(nonzero!(20u32)), clock.clone()); let ms = Duration::from_millis(1); for key in KEYS { @@ -144,7 +144,7 @@ fn hashmap_length() { fn hashmap_shrink_to_fit() { let clock = FakeRelativeClock::default(); // a steady rate of 3ms between elements: - let lim = RateLimiter::hashmap_with_clock(Quota::per_second(nonzero!(20u32)), &clock); + let lim = RateLimiter::hashmap_with_clock(Quota::per_second(nonzero!(20u32)), clock.clone()); let ms = Duration::from_millis(1); assert_eq!( diff --git a/governor/tests/middleware.rs b/governor/tests/middleware.rs index eeabf2ae..ed7ce24c 100644 --- a/governor/tests/middleware.rs +++ b/governor/tests/middleware.rs @@ -28,7 +28,7 @@ impl RateLimitingMiddleware<::Instant> for My #[test] fn changes_allowed_type() { let clock = FakeRelativeClock::default(); - let lim = RateLimiter::direct_with_clock(Quota::per_hour(nonzero!(1_u32)), &clock) + let lim = RateLimiter::direct_with_clock(Quota::per_hour(nonzero!(1_u32)), clock.clone()) .with_middleware::(); assert_eq!(Ok(666), lim.check()); assert_eq!(Err(()), lim.check()); @@ -37,7 +37,7 @@ fn changes_allowed_type() { #[test] fn state_information() { let clock = FakeRelativeClock::default(); - let lim = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(4u32)), &clock) + let lim = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(4u32)), clock.clone()) .with_middleware::(); assert_eq!( Ok(3), @@ -82,7 +82,7 @@ fn state_snapshot_tracks_quota_accurately() { let clock = FakeRelativeClock::default(); // First test - let lim = RateLimiter::direct_with_clock(quota, &clock) + let lim = RateLimiter::direct_with_clock(quota, clock.clone()) .with_middleware::(); assert_eq!(lim.check().unwrap().remaining_burst_capacity(), 1); diff --git a/governor/tests/proptests.rs b/governor/tests/proptests.rs index 4926e114..3beafaeb 100644 --- a/governor/tests/proptests.rs +++ b/governor/tests/proptests.rs @@ -42,7 +42,7 @@ fn cover_count_derives() { fn accurate_not_until() { proptest!(test_config(), |(capacity: Count, additional: Count, wait_time_parts: Count)| { let clock = FakeRelativeClock::default(); - let lb = RateLimiter::direct_with_clock(Quota::per_second(capacity.0), &clock); + let lb = RateLimiter::direct_with_clock(Quota::per_second(capacity.0), clock.clone()); let step = Duration::from_secs(1) / capacity.0.get(); // use up the burst capacity: From e9b2f547a815aea9d44e49d755b7ee01e817940c Mon Sep 17 00:00:00 2001 From: koshell Date: Sun, 5 May 2024 16:24:05 +1000 Subject: [PATCH 4/6] This test fails due to precisison on Windows. --- governor/src/clock/with_std.rs | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/governor/src/clock/with_std.rs b/governor/src/clock/with_std.rs index c7096046..472ddc1d 100644 --- a/governor/src/clock/with_std.rs +++ b/governor/src/clock/with_std.rs @@ -127,15 +127,32 @@ mod test { #[test] fn system_clock_impls_coverage() { - let one_ns = Nanos::new(1); + const NS: u64 = { + // SystemTime precision is OS-specific. + // For Windows it is only precise to 100ns. + // + // As far as I'm aware unix systems are typically precise to 1ns. + // + // See: https://doc.rust-lang.org/std/time/struct.SystemTime.html + // + // -- koshell + if cfg!(windows) { + 100 + } else { + 1 + } + }; + + let ns = Nanos::new(NS); let c = SystemClock::default(); let now = c.now(); - assert_ne!(now + one_ns, now); + + assert_ne!(now + ns, now); // Thankfully, we're not comparing two system clock readings // here so that ought to be safe, I think: - assert_eq!(one_ns, Reference::duration_since(&(now + one_ns), now)); + assert_eq!(ns, Reference::duration_since(&(now + ns), now)); assert_eq!( - Reference::saturating_sub(&(now + Duration::from_nanos(1)), one_ns), + Reference::saturating_sub(&(now + Duration::from_nanos(NS)), ns), now ); } From e7bd0ea25cb04f4ffe07c517408a81704166833a Mon Sep 17 00:00:00 2001 From: koshell Date: Sun, 5 May 2024 18:18:45 +1000 Subject: [PATCH 5/6] Fixed missing macro import --- governor/src/gcra.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/governor/src/gcra.rs b/governor/src/gcra.rs index 1fa7e65b..8328b25a 100644 --- a/governor/src/gcra.rs +++ b/governor/src/gcra.rs @@ -266,6 +266,8 @@ mod test { #[test] fn test_to_quota() { + use nonzero_ext::nonzero; + let quota = Quota { max_burst: nonzero!(32_u32), replenish_1_per: Duration::from_secs(3), From c02d006b9914a776166d84883cada624f75932bf Mon Sep 17 00:00:00 2001 From: koshell Date: Sun, 5 May 2024 18:20:58 +1000 Subject: [PATCH 6/6] enable all tests to run under Windows --- governor/Cargo.toml | 35 +++++++++++++++++++++++----------- governor/tests/memory_leaks.rs | 28 ++++++++++++++++++++++----- 2 files changed, 47 insertions(+), 16 deletions(-) diff --git a/governor/Cargo.toml b/governor/Cargo.toml index 972f5204..033309fa 100644 --- a/governor/Cargo.toml +++ b/governor/Cargo.toml @@ -25,17 +25,6 @@ harness = false [lib] bench = false -[dev-dependencies] -criterion = { version = "0.5.1", features = ["html_reports"] } -tynm = "0.1.4" -crossbeam = "0.8.0" -futures = "0.3.5" -proptest = "1.0.0" -all_asserts = "2.2.0" - -[target.'cfg(unix)'.dev-dependencies] -libc = "0.2.70" - [features] default = ["std", "dashmap", "jitter", "quanta"] quanta = ["dep:quanta"] @@ -64,3 +53,27 @@ cfg-if = "1.0" # To ensure we don't pull in vulnerable smallvec, see https://github.com/antifuchs/governor/issues/60 smallvec = "1.6.1" + +[dev-dependencies] +criterion = { version = "0.5.1", features = ["html_reports"] } +tynm = "0.1.4" +crossbeam = "0.8.0" +futures = "0.3.5" +proptest = "1.0.0" +all_asserts = "2.2.0" + +[target.'cfg(unix)'.dev-dependencies] +libc = "0.2.70" + +[target.'cfg(windows)'.dev-dependencies] +windows = { version = "0.56.0", features = ["System_Diagnostics"] } + +# Due to #![deny(warnings)] you can't compile the crate without these lints disabled. +# These are simple to resolve lints, it's up to Andreas Fuchs if they should be resolved. +# +# -- koshell +[lints.clippy] +default_constructed_unit_structs = "allow" +single_char_pattern = "allow" +len_zero = "allow" +clone_on_copy = "allow" diff --git a/governor/tests/memory_leaks.rs b/governor/tests/memory_leaks.rs index caf639c2..768db80c 100755 --- a/governor/tests/memory_leaks.rs +++ b/governor/tests/memory_leaks.rs @@ -1,19 +1,37 @@ #![cfg(feature = "std")] -#![cfg(unix)] -// This test uses procinfo, so can only be run on Linux. +#[cfg(unix)] extern crate libc; use all_asserts::*; use governor::{Quota, RateLimiter}; use nonzero_ext::*; +use std::convert::TryInto; use std::sync::Arc; use std::thread; fn resident_memory_size() -> i64 { - let mut out: libc::rusage = unsafe { std::mem::zeroed() }; - assert!(unsafe { libc::getrusage(libc::RUSAGE_SELF, &mut out) } == 0); - out.ru_maxrss + #[cfg(unix)] + { + let mut out: libc::rusage = unsafe { std::mem::zeroed() }; + assert!(unsafe { libc::getrusage(libc::RUSAGE_SELF, &mut out) } == 0); + out.ru_maxrss + } + + #[cfg(windows)] + { + (windows::System::Diagnostics::ProcessDiagnosticInfo::GetForCurrentProcess() + .unwrap() + .MemoryUsage() + .unwrap() + .GetReport() + .unwrap() + .WorkingSetSizeInBytes() + .unwrap() + / 1024) // B => KiB + .try_into() + .unwrap() + } } const LEAK_TOLERANCE: i64 = 1024 * 1024 * 10;