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

rec: add bigger (and v6 enabled) bulk test on ubicloud #15018

Merged
merged 3 commits into from
Jan 10, 2025

Conversation

omoerbeek
Copy link
Member

@omoerbeek omoerbeek commented Jan 7, 2025

Short description

On ubicloud we do not run in a container any more, as it lacks v6 connectivity. We are running a debian executable on ubuntu, this is not causing any real issues (so far) except for some boost dependencies version mismatches. Luckily the boost packages required are available on ubuntu 24.04.

Current state of affairs:

  • We run mini (100 names) tsan and ubsan+asan tests on GH (v4 only)
  • We run two medium sized (50k names) ubsan + asan tests on ubicloud (v4 only and v6 enabled)

Big (or medium) tsan tests do not work as they get killed due to excessive memory requirements (many TB virtual plus many GB resident).

Plan is to add a bigger version of the bulk test in daily. That one should use a auto-built recursor package.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

@coveralls
Copy link

coveralls commented Jan 7, 2025

Pull Request Test Coverage Report for Build 12706379885

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 33 unchanged lines in 9 files lost coverage.
  • Overall coverage increased (+0.01%) to 64.86%

Files with Coverage Reduction New Missed Lines %
modules/gpgsqlbackend/gpgsqlbackend.cc 1 88.62%
pdns/pollmplexer.cc 1 83.66%
pdns/recursordist/test-rec-tcounters_cc.cc 2 95.77%
pdns/recursordist/sortlist.cc 2 74.12%
pdns/tcpiohandler.cc 2 68.18%
pdns/rcpgenerator.cc 3 90.73%
pdns/packethandler.cc 5 72.4%
pdns/recursordist/rec-tcpout.cc 8 50.79%
pdns/recursordist/syncres.cc 9 80.11%
Totals Coverage Status
Change from base Build 12706238396: 0.01%
Covered Lines: 126247
Relevant Lines: 163913

💛 - Coveralls

@omoerbeek omoerbeek force-pushed the rec-bulktest-ubicloud branch 16 times, most recently from 843748d to edc8f02 Compare January 8, 2025 10:38
@omoerbeek omoerbeek force-pushed the rec-bulktest-ubicloud branch from edc8f02 to 2c867d9 Compare January 8, 2025 10:58
@omoerbeek omoerbeek changed the title rec: start bulk test on ubicloud (experiment) rec: add bigger (and v6 enabled) bulk test on ubicloud Jan 8, 2025
@omoerbeek omoerbeek requested review from Habbie and romeroalx January 8, 2025 11:07
name: 'test rec ubicloud bulk'
needs:
- build-recursor
runs-on: ubicloud-standard-8-ubuntu-2404
Copy link
Member

Choose a reason for hiding this comment

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

As this job uses Ubicloud runners, which may impact forks, I suggest adding a repository variable to check if Ubicloud runners are available. Something like BULK_TESTS_USE_UBICLOUD=1 (or a much better name) as a condition for running the job:

...
if: ${{ vars.BULK_TESTS_USE_UBICLOUD == '1' }}
...

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, but how do I set this var in our own repo?

Copy link
Member

Choose a reason for hiding this comment

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

Variables are added at the repository level in Settings -> Secrets and Variables -> Actions -> Variables (tab) -> New Repository Variable

This has to be done by someone with enough privileges. I cannot see Settings for this repository, only for my fork. I have asked @Habbie for this in the past.

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 can do that

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I've set REC_BULKTEST_USE_UBICLOUD to 1, but the job is now skipped....

Copy link
Member Author

Choose a reason for hiding this comment

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

It's because vars are not set on PRs from a fork.

Copy link
Member

@romeroalx romeroalx left a comment

Choose a reason for hiding this comment

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

LGTM!

@omoerbeek omoerbeek force-pushed the rec-bulktest-ubicloud branch from 69e680b to 196a9a7 Compare January 10, 2025 09:00
with c.cd('regression-tests'):
c.run('curl -LO http://s3-us-west-1.amazonaws.com/umbrella-static/top-1m.csv.zip')
c.run('curl --no-progress-meter -LO http://s3-us-west-1.amazonaws.com/umbrella-static/top-1m.csv.zip')
Copy link
Member

Choose a reason for hiding this comment

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

I recently discovered --process=dot:giga (or mega depending on size) which you might like

@omoerbeek omoerbeek merged commit 23a04bd into PowerDNS:master Jan 10, 2025
81 checks passed
@omoerbeek omoerbeek deleted the rec-bulktest-ubicloud branch January 10, 2025 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants