MOD-7695: Decide Ubuntu Runner Using Variable by kei-nan · Pull Request #4975 · RediSearch/RediSearch

kei-nan · 2024-08-29T14:45:02Z

Decide which ubuntu runner to use based on a github variable

A clear and concise description of what the PR is solving, including:

We always use ubuntu-latest
Use the runner from the RUNS_ON_UBUNTU_LATEST variable
Allow to choose which ubuntu latest runner without modifying our code

Which issues this PR fixes
2. MOD-7695

Main objects this PR modified

Workflow Files

Mark if applicable

This PR introduces API changes
This PR introduces serialization changes

.github/workflows/benchmark-flow.yml

raz-mon · 2024-08-29T15:06:23Z

@kei-nan When\Why should this be changed from latest in the global level?
If you want to run a specific action with a non-latest version you can always do it manually.

codecov · 2024-08-29T15:09:27Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.15%. Comparing base (baaf2fe) to head (8cc9322).
Report is 2 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4975   +/-   ##
=======================================
  Coverage   86.15%   86.15%           
=======================================
  Files         192      192           
  Lines       34229    34235    +6     
=======================================
+ Hits        29490    29496    +6     
  Misses       4739     4739

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions · 2024-09-01T08:36:10Z

Backport failed for 2.8, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 2.8
git worktree add -d .worktree/backport-4975-to-2.8 origin/2.8
cd .worktree/backport-4975-to-2.8
git switch --create backport-4975-to-2.8
git cherry-pick -x b08373e45d63e67d70f40c45f39d85fba36c3d0e

github-actions · 2024-09-01T08:36:12Z

Backport failed for 2.6, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 2.6
git worktree add -d .worktree/backport-4975-to-2.6 origin/2.6
cd .worktree/backport-4975-to-2.6
git switch --create backport-4975-to-2.6
git cherry-pick -x b08373e45d63e67d70f40c45f39d85fba36c3d0e

github-actions · 2024-09-01T08:36:14Z

Git push to origin failed for 2.10 with exitcode 1

* * initial commit * * code review

* Move coord src into general module's src * remove internal src dir in coord * remove internal src dir in coord - now with cmake * WIP * single build path - wip * WIP * remove split * make things work - WIP - clean makefile, move pack * fix pack, move coord unit tests wip: resolve conflicts, fix tests, libuv? * fix conflicts, split test * remove build test step from CI * fixes * remove COORD=0 for coverage * revert extract module symbols * use foreach loop for coordinator unit tests * Addressing Dvir's CR - simplifying test deps and removing CMAKE_FILES var that seems useless * fix CMAKE_COORD var * MOD-7596: Add mirroring support (#4937) * * initial commit * * Use RediSearch branch mirroring fork * * Code Review - Round #1 * Change maxPending requests when changing the number of connections (#4972) change the number of maxPending requests when changing the number of connections * Remove RS_COORDINATOR flag and its associated code, fix bug in makefile for lite and static * fix to allow building as LITE * fix path in unit-tests to community * MOD-7695: Decide Ubuntu Runner Using Variable (#4975) * * initial commit * * code review * Enable MT in comunity eddition by default - [MOD-7160] (#4974) * remove compilation flag (wip) * finish removing compilation flag * remove from tests * split build for lite * MOD-7706 fix ramp-packer version (#4980) ramp-packer 2.5.10 * HNSW vector index - handle inplace deletion after async [MOD-7643] (#4964) * Fix inplace deletion in tiered index in vecsim * remove excessive thread iteration log, finalize test * update vecsim version * update vecsim version - fixing bug found in test vecsim with json multi * remove print from test * better logging * test - print warning always * test2 - print warning DEBUG AND CI ENV VARS * use force for env vars * try print instead * print for GHA annotation * remove light build flag, keep lite as an option fro backward compatibility * Fix warning * use llvm 18 * try to include in cmake cpp test dir * try build without special includes for apple * set apple test * Set APPLE flag for MACOS * debug print * MOD-7737: Add relaxation flags to Rocky9 installations (#4989) Add relaxing installation flags to Rocky platform * MOD-7646 cmake fetch boost if not exists (#4984) cmake fetch boost if not exists * MOD-7465 MOD-7466: Add ACL support for RediSearch, phase 1 (#4957) * Support ACL for RediSearch commands - wip * Register RediSearch commands to redis ACL categories * Add tests * Small fix * Add coordinator commands registration * Register underscored commands to ACL as well * Remove dangerous from INFO * Skip test conditioned on redis version * Fix categories according to new evaluation * Remove comments * Fix * Address review * Remove TODOs * Replace macro with function * Wrap with RM_TRY * Fix unit-tests * Touchup * Address review * Address review * Address review * Bump sanitizer and Valgrind to use 7.4 * Address review * Touchups * Remove TODOs * Fix test * Address review * Move hack to src * try removing visibility * try remove visibility for coord only * apply changes as patch after rebasing from new master * revert visibility hidden * revert visibility hidden * revert * add verbosity for debug * add init-rm to cpp tests * Copy init-rm in cpptests * clean * avoid -executable in mac * move REDISMODULE_MAIN to module-init.c * try removing init-rm from cpp tests * update vecsim * unify module.c files from coord so that "onload" is on the module not coord * remove init-rm * bring back sdsfree in unit test, link hiredis/sds with extern c * remove coord_module.h, align vecsim deps * remove sds from rmutil * remove RMUTIL_NO_SDS totally --------- Co-authored-by: kei-nan <[email protected]> Co-authored-by: GuyAv46 <[email protected]> Co-authored-by: DvirDukhan <[email protected]> Co-authored-by: Raz Monsonego <[email protected]>

* * initial commit * * code review

* MOD-7596: Add mirroring support (#4937) * * initial commit * * Use RediSearch branch mirroring fork * * Code Review - Round #1 * MOD-7695: Decide Ubuntu Runner Using Variable (#4975) * * initial commit * * code review * run against redis 8 * rebase 8.0 --------- Co-authored-by: kei-nan <[email protected]>

* MOD-7596: Add mirroring support (#4937) * * initial commit * * Use RediSearch branch mirroring fork * * Code Review - Round #1 * MOD-7695: Decide Ubuntu Runner Using Variable (#4975) * * initial commit * * code review * Dvirdu_set_7.99.0_version_and_test_unstable (#4997) * 7.99.0 * redis unstable tests * CP single build path * Fix benchmark to use new build path (#5026) * Fix benchmark to use new build path * Take Raz's fix for benchmarks * fix default module path * make benchmark fail * fix path * fail external flow * remove fail fast and continue on error * addressing CR * Splitting the standalone work among 6 groups. Only profile if explictly enabled. Only run >4 primaries if extended if enabled. * Specify name of benchmark step on GH flow * Using perf-cto-base-image-ubuntu20.04-redis-7.4.0-tuned-latency on spot instances as well. * Fixed RedisJSON module path on benchmark setup. Enforcing ReJSON module to be present * Changing from 6 to 9 oss-standalone runners to achieve a time for full feedback < 30min * MODULE_FILE not working on Prepare ReJSON Module: using default location * Using redisbench-admin >= 0.11.3 due to multi-module issue * Using redisbench-admin >= 0.11.3 due to multi-module issue * Added missing --allowed-setups info for oss-standalone config * redisbench-admin >= 0.11.4 * Remove vecsim-ann-benchmarks_LOAD_1c_redisearch-hnsw_glove-200-angular_M-4.yml * Using redisbench-admin >= 0.11.5 due to ssh timeout issues on heavy benchmarks or heavy prunning at the end * Increasing concurrency of standalone benchmarks to reduce time to feedback * redisbench-admin >= 0.11.6. removed unstable test vecsim-ann-benchmarks_QUERY_1c_redisearch-hnsw_glove-200-angular_M-4 * redisbench-admin >= 0.11.7. running cluster tests faster and also ensuring we use a redisbench-admin with keepalive on ssh tunnels * Removed search-ftsb-10K-enwiki_abstract-hashes-term-withoutsuffix-trie from cluster tests due to keyspace error check * Removed search-ftsb-1M-enwiki_abstract-hashes-fulltext-simple-1word-query-non-sortable from cluster tests due to client error. created RediSearch/ftsb#97 * Not running *.rdb based benchmarks on cluster due to orphaned keys (redis/redis#9506) --------- Co-authored-by: fcostaoliveira <[email protected]> * rebase 8.0 --------- Co-authored-by: kei-nan <[email protected]> Co-authored-by: DvirDukhan <[email protected]> Co-authored-by: fcostaoliveira <[email protected]>

* Fix NOSTEM * Fix testNoStem * Check spec is not NULL * Add comments to testNoStem * MOD-7596: Add mirroring support (#4937) * * initial commit * * Use RediSearch branch mirroring fork * * Code Review - Round #1 * Change maxPending requests when changing the number of connections (#4972) change the number of maxPending requests when changing the number of connections * MOD-7695: Decide Ubuntu Runner Using Variable (#4975) * * initial commit * * code review * Enable MT in comunity eddition by default - [MOD-7160] (#4974) * remove compilation flag (wip) * finish removing compilation flag * remove from tests * MOD-7706 fix ramp-packer version (#4980) ramp-packer 2.5.10 * Fix expansion for modifier lists * Delete event-mirror.yml it is not part of this PR * Simplify bit masking * Update src/query.c Co-authored-by: Omer Shadmi <[email protected]> * Update src/ext/default.c Co-authored-by: Omer Shadmi <[email protected]> --------- Co-authored-by: kei-nan <[email protected]> Co-authored-by: GuyAv46 <[email protected]> Co-authored-by: DvirDukhan <[email protected]> Co-authored-by: Omer Shadmi <[email protected]>

* wip * wip * wip * wip * delete * added get-redis input to sanitzier * removed no draft for sanitizer * added mode for llvm install * restored build redisjson * wip * after rebase * fixed typo * llvm_version * restored install boost * fixed? * set clang and llvm version * take llvm version from env * fixed build redisjson * don't run json on sanitizer * fixed task-test * removed flow-sanitizer * moved rust and llvm to test_deps. fixed llvm params order * fixed PR comments * remove unused * upload logs if canceled * tmp add test * also on coord * try explicitly set flags * MOD-7596: Add mirroring support (#4937) * * initial commit * * Use RediSearch branch mirroring fork * * Code Review - Round #1 * MOD-7695: Decide Ubuntu Runner Using Variable (#4975) * * initial commit * * code review * added LD flag * trying the whole tests * delete event mirror * try a clean container * add -y * add curl to jammy * single tests for logs * add SAN to cmake defs * shared linker flags * fixed a typo * build redis with clang * Add cflags to redis for sanitizer * remove = * skip json in new json test, split phases in build redis (install after) * Switch position Build_tls and sanitizer * typo in sanitizer * run full test suite * Set sanitizer to run against latest redis with static libstdc++ * Run tests with json as well * removed jammy from PR event * bring back jammy * added jammy to merge queue and nightly * added test_config to merge queue * install LLVM only on sanitizer * added working directory * curl * remove curl from amazon linux 2023 * bring back install llvm * put rust in bashrc * add rest to path - another attempt * move rust to common installations --------- Co-authored-by: kei-nan <[email protected]> Co-authored-by: alon-reshef <[email protected]> Co-authored-by: alonre24 <[email protected]>

* Fix NOSTEM * Fix testNoStem * Check spec is not NULL * Add comments to testNoStem * MOD-7596: Add mirroring support (#4937) * * initial commit * * Use RediSearch branch mirroring fork * * Code Review - Round #1 * Change maxPending requests when changing the number of connections (#4972) change the number of maxPending requests when changing the number of connections * MOD-7695: Decide Ubuntu Runner Using Variable (#4975) * * initial commit * * code review * Enable MT in comunity eddition by default - [MOD-7160] (#4974) * remove compilation flag (wip) * finish removing compilation flag * remove from tests * MOD-7706 fix ramp-packer version (#4980) ramp-packer 2.5.10 * Fix expansion for modifier lists * Delete event-mirror.yml it is not part of this PR * Simplify bit masking * Update src/query.c Co-authored-by: Omer Shadmi <[email protected]> * Update src/ext/default.c Co-authored-by: Omer Shadmi <[email protected]> --------- Co-authored-by: kei-nan <[email protected]> Co-authored-by: GuyAv46 <[email protected]> Co-authored-by: DvirDukhan <[email protected]> Co-authored-by: Omer Shadmi <[email protected]> (cherry picked from commit 54af5c8)

MOD-7634: Fix NOSTEM (#4956) * Fix NOSTEM * Fix testNoStem * Check spec is not NULL * Add comments to testNoStem * MOD-7596: Add mirroring support (#4937) * * initial commit * * Use RediSearch branch mirroring fork * * Code Review - Round #1 * Change maxPending requests when changing the number of connections (#4972) change the number of maxPending requests when changing the number of connections * MOD-7695: Decide Ubuntu Runner Using Variable (#4975) * * initial commit * * code review * Enable MT in comunity eddition by default - [MOD-7160] (#4974) * remove compilation flag (wip) * finish removing compilation flag * remove from tests * MOD-7706 fix ramp-packer version (#4980) ramp-packer 2.5.10 * Fix expansion for modifier lists * Delete event-mirror.yml it is not part of this PR * Simplify bit masking * Update src/query.c Co-authored-by: Omer Shadmi <[email protected]> * Update src/ext/default.c Co-authored-by: Omer Shadmi <[email protected]> --------- Co-authored-by: kei-nan <[email protected]> Co-authored-by: GuyAv46 <[email protected]> Co-authored-by: DvirDukhan <[email protected]> Co-authored-by: Omer Shadmi <[email protected]> (cherry picked from commit 54af5c8) Co-authored-by: nafraf <[email protected]>

* WIP: Config parameters * Refactor config register * MOD-7596: Add mirroring support (#4937) * * initial commit * * Use RediSearch branch mirroring fork * * Code Review - Round #1 * Change maxPending requests when changing the number of connections (#4972) change the number of maxPending requests when changing the number of connections * Test boolean parameters * Fix boolean params default values * MOD-7695: Decide Ubuntu Runner Using Variable (#4975) * * initial commit * * code review * Enable MT in comunity eddition by default - [MOD-7160] (#4974) * remove compilation flag (wip) * finish removing compilation flag * remove from tests * MOD-7706 fix ramp-packer version (#4980) ramp-packer 2.5.10 * HNSW vector index - handle inplace deletion after async [MOD-7643] (#4964) * Fix inplace deletion in tiered index in vecsim * remove excessive thread iteration log, finalize test * update vecsim version * update vecsim version - fixing bug found in test vecsim with json multi * remove print from test * better logging * test - print warning always * test2 - print warning DEBUG AND CI ENV VARS * use force for env vars * try print instead * print for GHA annotation * Add two immutable params * Fix max-doctablesize name * MOD-7737: Add relaxation flags to Rocky9 installations (#4989) Add relaxing installation flags to Rocky platform * MOD-7646 cmake fetch boost if not exists (#4984) cmake fetch boost if not exists * MOD-7465 MOD-7466: Add ACL support for RediSearch, phase 1 (#4957) * Support ACL for RediSearch commands - wip * Register RediSearch commands to redis ACL categories * Add tests * Small fix * Add coordinator commands registration * Register underscored commands to ACL as well * Remove dangerous from INFO * Skip test conditioned on redis version * Fix categories according to new evaluation * Remove comments * Fix * Address review * Remove TODOs * Replace macro with function * Wrap with RM_TRY * Fix unit-tests * Touchup * Address review * Address review * Address review * Bump sanitizer and Valgrind to use 7.4 * Address review * Touchups * Remove TODOs * Fix test * Address review * Move hack to src * WIP: Register numeric params * WIP: Config numeric params * WIP: CONFIG value precedence * Complete boolean parameters * Test search.partial-indexed-docs * Fix string configs * Add index-cursor-limit config * Fix unit tests * Create cluster configuration params * Fix cluster configuration params * Test MODULE LOADEX numeric config * WIP: Test MODULE LOADEX bool config * Fix set_no_gc() * Fix string config parameters * Test MODULE LOADEX enum config * Fix set_workers() * Fix numeric parameter limit * Test boolean parameters with config file * Test with config file: numeric, enum, string * Add max-aggregate-results and max-search-results * tidy up * Add more tests * Use REDISMODULE_CONFIG_UNPREFIXED flag * Revert "Use REDISMODULE_CONFIG_UNPREFIXED flag" This reverts commit b1985fc. * Use common setfn, getfn for numeric params * Fix mem leak 1 * Fix mem leak 2 - string config * Disable immutable limits tests * Use RLTest >= 0.7.14 * Check version before loading config * Add testConfigAPILoadTimeNumericParams() * Fix unit-tests * Refactor config API function parameters for consistency * Use privdata in numeric config params * Refactor boolean config registration and clean up unused macros * Refactor configuration setter functions to improve consistency * Refactor coord config functions * Refactor config functions for improved clarity and consistency * search-gc-scan-size is Load-Time immutable * Fixes from code review * Remove parameter modified validation * Fix search-ext-load and search-friso-ini tests * Remove global config vars * Remove str validation from get_string_config() * Update default configuration to initialize extLoad and frisoIni as empty strings * MOD-7613: Add search-oss-global-password (#5237) 1. Configure the module parameter search-oss-global-password which is equivalent to the previous OSS_GLOBAL_PASSWORD module parameter. 2. Configure the module parameter search-oss-acl-username which is equivalent to the previous OSS_ACL_USERNAME --------- Co-authored-by: kei-nan <[email protected]> Co-authored-by: GuyAv46 <[email protected]> Co-authored-by: DvirDukhan <[email protected]> Co-authored-by: alonre24 <[email protected]> Co-authored-by: Raz Monsonego <[email protected]> Co-authored-by: oshadmi <[email protected]>

* initial commit

1ad2763

kei-nan requested review from GuyAv46 and raz-mon August 29, 2024 14:45

kei-nan self-assigned this Aug 29, 2024

kei-nan added backport 2.8 backport 2.6 backport 2.10 labels Aug 29, 2024

raz-mon reviewed Aug 29, 2024

View reviewed changes

.github/workflows/benchmark-flow.yml Outdated Show resolved Hide resolved

* code review

8cc9322

kei-nan requested a review from raz-mon August 29, 2024 15:34

kei-nan enabled auto-merge August 29, 2024 15:39

raz-mon approved these changes Aug 29, 2024

View reviewed changes

kei-nan marked this pull request as draft September 1, 2024 06:47

auto-merge was automatically disabled September 1, 2024 06:47
Pull request was converted to draft

kei-nan marked this pull request as ready for review September 1, 2024 06:47

kei-nan enabled auto-merge September 1, 2024 06:55

kei-nan added this pull request to the merge queue Sep 1, 2024

Merged via the queue into master with commit b08373e Sep 1, 2024

kei-nan deleted the master_jk_ubuntu_runner_from_variable branch September 1, 2024 08:35

kei-nan added a commit that referenced this pull request Sep 8, 2024

MOD-7695: Decide Ubuntu Runner Using Variable (#4975)

9cc5edd

* * initial commit * * code review

DvirDukhan pushed a commit that referenced this pull request Sep 10, 2024

MOD-7695: Decide Ubuntu Runner Using Variable (#4975)

54be2d5

* * initial commit * * code review

DvirDukhan pushed a commit that referenced this pull request Sep 16, 2024

MOD-7695: Decide Ubuntu Runner Using Variable (#4975)

bb15c4a

* * initial commit * * code review

alonre24 pushed a commit that referenced this pull request Sep 24, 2024

MOD-7695: Decide Ubuntu Runner Using Variable (#4975)

2543c06

* * initial commit * * code review

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MOD-7695: Decide Ubuntu Runner Using Variable#4975

MOD-7695: Decide Ubuntu Runner Using Variable#4975
kei-nan merged 2 commits intomasterfrom
master_jk_ubuntu_runner_from_variable

kei-nan commented Aug 29, 2024

Uh oh!

Uh oh!

raz-mon commented Aug 29, 2024

Uh oh!

codecov bot commented Aug 29, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Sep 1, 2024

Uh oh!

github-actions bot commented Sep 1, 2024

Uh oh!

github-actions bot commented Sep 1, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

kei-nan commented Aug 29, 2024

Uh oh!

Uh oh!

raz-mon commented Aug 29, 2024

Uh oh!

codecov bot commented Aug 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Sep 1, 2024

Uh oh!

github-actions bot commented Sep 1, 2024

Uh oh!

github-actions bot commented Sep 1, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Aug 29, 2024 •

edited

Loading