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.

Sign up for GitHub

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

Jump to bottom

rlm_redis_ippool: break out lua scripts as defacto schema (take 2) #3467

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

rlm_redis_ippool: break out lua scripts as defacto schema (take 2) #3467

wants to merge 13 commits into from

Conversation

jimdigriz
Copy link
Contributor

Resurrected the work from #2626

Summary of changes:

  • fix redis_ippool unit tests
  • extracted lua code into separate files (can be treated as a schema for redis_ippool)
  • documented the namespace usage in Redis
  • moved rlm_redis_ippool.c:ippool_script to cluster.c:fr_redis_script so to remove code duplication
  • moved IP iteration into Lua (partially addresses concerns in Speed up redis IP pool name listing on high latency links #1744)
    • includes 65k item guard for range based operations
  • support sticky IPs (device keys live for 10x lease time)
  • Misc other unrelated changes

Changes since last PR:

  • broken the patch set further to show evolution of Lua scripts
  • removed the netinfo feature that was added (generic KV store)
  • made lease info use arrays instead of JSON
  • retained gateway/counter entries
  • 'modify' retained (reuses 'add')

@jimdigriz jimdigriz force-pushed the rlm_redis_ippool_lua branch from 91ab1d4 to 3e10f1f Compare May 24, 2020 11:35
@lgtm-com
Copy link

lgtm-com bot commented May 24, 2020

This pull request introduces 1 alert when merging 3e10f1f into ef9fb73 - view on LGTM.com

new alerts:

  • 1 for FIXME comment

@jimdigriz jimdigriz force-pushed the rlm_redis_ippool_lua branch from 3e10f1f to 2b18b97 Compare May 24, 2020 14:01
@jimdigriz
Copy link
Contributor Author

jimdigriz commented May 24, 2020 •
edited
Loading

Now dropped after fix.

The cache_rbtree unit tests remains disable until fixed upstream.

@lgtm-com
Copy link

lgtm-com bot commented May 24, 2020

This pull request introduces 1 alert when merging 2b18b97 into 6569b39 - view on LGTM.com

new alerts:

  • 1 for FIXME comment

@jimdigriz jimdigriz force-pushed the rlm_redis_ippool_lua branch 2 times, most recently from caed00d to 412f6d1 Compare May 24, 2020 17:15
@jimdigriz
Copy link
Contributor Author

The cache_rbtree unit tests remains disable until fixed upstream.

Now dropped after fix.

@lgtm-com
Copy link

lgtm-com bot commented May 24, 2020

This pull request introduces 1 alert when merging 412f6d1 into eada45a - view on LGTM.com

new alerts:

  • 1 for FIXME comment

@jimdigriz jimdigriz force-pushed the rlm_redis_ippool_lua branch from 412f6d1 to 46997d0 Compare May 25, 2020 05:50
@lgtm-com
Copy link

lgtm-com bot commented May 25, 2020

This pull request introduces 1 alert when merging 46997d0 into 7cad2fc - view on LGTM.com

new alerts:

  • 1 for FIXME comment

@jimdigriz jimdigriz force-pushed the rlm_redis_ippool_lua branch from 46997d0 to 46c6db4 Compare May 25, 2020 15:42
@lgtm-com
Copy link

lgtm-com bot commented May 25, 2020

This pull request introduces 1 alert when merging 46c6db4 into 0d6800f - view on LGTM.com

new alerts:

  • 1 for FIXME comment

@jimdigriz jimdigriz force-pushed the rlm_redis_ippool_lua branch from 46c6db4 to 7bf4d04 Compare May 25, 2020 23:48
@lgtm-com
Copy link

lgtm-com bot commented May 26, 2020

This pull request introduces 1 alert when merging 7bf4d04 into 261d0b9 - view on LGTM.com

new alerts:

  • 1 for FIXME comment

@jimdigriz jimdigriz force-pushed the rlm_redis_ippool_lua branch from 7bf4d04 to 720f734 Compare May 26, 2020 06:08
@lgtm-com
Copy link

lgtm-com bot commented May 26, 2020

This pull request introduces 1 alert when merging 720f734 into 436ee70 - view on LGTM.com

new alerts:

  • 1 for FIXME comment

@jimdigriz jimdigriz force-pushed the rlm_redis_ippool_lua branch from 720f734 to b81dd95 Compare May 28, 2020 22:45
@lgtm-com
Copy link

lgtm-com bot commented May 28, 2020

This pull request introduces 1 alert when merging b81dd95 into c6fcf29 - view on LGTM.com

new alerts:

  • 1 for FIXME comment

@jimdigriz jimdigriz force-pushed the rlm_redis_ippool_lua branch from b81dd95 to 945dd8e Compare May 29, 2020 11:02
@lgtm-com
Copy link

lgtm-com bot commented May 29, 2020

This pull request introduces 1 alert when merging 945dd8e into 6971a29 - view on LGTM.com

new alerts:

  • 1 for FIXME comment

@jimdigriz jimdigriz force-pushed the rlm_redis_ippool_lua branch from 945dd8e to 78a6243 Compare May 30, 2020 14:19
@lgtm-com
Copy link

lgtm-com bot commented May 30, 2020

This pull request introduces 1 alert when merging 78a6243 into 7af386e - view on LGTM.com

new alerts:

  • 1 for FIXME comment

@jimdigriz
Copy link
Contributor Author

On my laptop (from the build-docker10 image) the unit tests work for me:

[snipped]
MODULE-TEST python auth_multi_inst
MODULE-TEST python auth_cext_compat
MODULE-TEST redis cluster_node_fail
MODULE-TEST redis cluster_key
OK: redis.test
MODULE-TEST redis_ippool update_alloc
MODULE-TEST redis_ippool alloc
MODULE-TEST redis_ippool release
MODULE-TEST redis_ippool update
MODULE-TEST redis_ippool pool_tool_modify
MODULE-TEST redis_ippool pool_tool_delete
MODULE-TEST redis_ippool pool_tool_release
OK: redis_ippool.test
MODULE-TEST sql_sqlite acct_2_stop
MODULE-TEST sql_sqlite map
[snipped]

No idea why Travis is grumpy, if someone can get me debug access or have a look themselves?

It is really strange as Travis has no problems doing the redis tests but for the redis ippool tests it fails as the cluster does not start up properly; remap keeps returning bad-input.

@jimdigriz jimdigriz force-pushed the rlm_redis_ippool_lua branch from 78a6243 to 4f21c8e Compare May 30, 2020 19:38
@lgtm-com
Copy link

lgtm-com bot commented May 30, 2020

This pull request introduces 1 alert when merging 4f21c8e into ff0ed5a - view on LGTM.com

new alerts:

  • 1 for FIXME comment

@jimdigriz jimdigriz force-pushed the rlm_redis_ippool_lua branch from 4f21c8e to 2dfa8cd Compare May 31, 2020 06:31
@lgtm-com
Copy link

lgtm-com bot commented May 31, 2020

This pull request introduces 1 alert when merging 2dfa8cd into f39016d - view on LGTM.com

new alerts:

  • 1 for FIXME comment

@jimdigriz jimdigriz force-pushed the rlm_redis_ippool_lua branch from 2dfa8cd to 7aaa897 Compare June 9, 2020 16:45
@lgtm-com
Copy link

lgtm-com bot commented Jun 9, 2020

This pull request introduces 1 alert when merging 7aaa897 into 69f1575 - view on LGTM.com

new alerts:

  • 1 for FIXME comment

jimdigriz added 2 commits July 7, 2020 12:17
@jimdigriz
replace redis_ippool C based IP iterator with Lua one (improves FreeR…
d85ac8f 
…ADIUS#1744)

We no longer filter skip the 'broadcast' address as some topologies
it can be used and it is trivial to remove after adding.
@jimdigriz
include notes about the schema used inside redis for redis_ippool
4cde0a2
@jimdigriz jimdigriz force-pushed the rlm_redis_ippool_lua branch from 7aaa897 to 4cde0a2 Compare July 7, 2020 11:18
@lgtm-com
Copy link

lgtm-com bot commented Jul 7, 2020

This pull request introduces 1 alert when merging 4cde0a2 into 7a1293c - view on LGTM.com

new alerts:

  • 1 for FIXME comment

@arr2036 arr2036 force-pushed the master branch 2 times, most recently from 5b9b8f2 to c1abc5e Compare October 23, 2020 02:53
@mcnewton mcnewton added the v4.0.x meta: relates to the v4.0.x branch label Oct 8, 2021
@arr2036 arr2036 added on hold state: PR acceptance is pending completion of other work usability category: relates to how easy (or difficult) a feature is to use labels Mar 24, 2022
@arr2036 arr2036 force-pushed the master branch 9 times, most recently from 06700bb to 8dc35c2 Compare May 26, 2023 15:46
@arr2036 arr2036 force-pushed the master branch 4 times, most recently from 76ed4dd to 48ca41e Compare May 16, 2024 00:49
@arr2036 arr2036 force-pushed the master branch 2 times, most recently from 78c97fd to 1a249f5 Compare September 2, 2024 00:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
on hold state: PR acceptance is pending completion of other work usability category: relates to how easy (or difficult) a feature is to use v4.0.x meta: relates to the v4.0.x branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jimdigriz @arr2036 @mcnewton