-
Notifications
You must be signed in to change notification settings - Fork 755
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
Add support for MustObeyClient Module API #1582
Conversation
Signed-off-by: KarthikSubbarao <[email protected]>
Signed-off-by: KarthikSubbarao <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## unstable #1582 +/- ##
============================================
+ Coverage 70.94% 71.00% +0.05%
============================================
Files 121 121
Lines 65132 65181 +49
============================================
+ Hits 46207 46279 +72
+ Misses 18925 18902 -23
|
Signed-off-by: KarthikSubbarao <[email protected]>
Signed-off-by: KarthikSubbarao <[email protected]>
Signed-off-by: KarthikSubbarao <[email protected]>
Signed-off-by: Karthik Subbarao <[email protected]>
Signed-off-by: Karthik Subbarao <[email protected]>
Signed-off-by: Karthik Subbarao <[email protected]>
@valkey-io/core-team Please vote. This is a pretty straight forward API to give modules the ability to quickly check if they should apply the command and ignore checks because it's coming from a primary or AOF. Worth mentioning that this is more performant than ValkeyModule_GetContextFlags, since that command also checks for memory bounds. |
There is a timing issue in the test: ``` *** [err]: module RM_Call of expired key propagation in tests/unit/moduleapi/propagate.tcl Expected '1' to be equal to '2' (context: type eval line 27 cmd {assert_equal [$replica propagate-test.obeyed] 2} proc ::test) ``` We should wait for sync, otherwise the replica may not be fully synchronized before checking. The test was introduced in valkey-io#1582. Signed-off-by: Binbin <[email protected]>
There is a timing issue in the test: ``` *** [err]: module RM_Call of expired key propagation in tests/unit/moduleapi/propagate.tcl Expected '1' to be equal to '2' (context: type eval line 27 cmd {assert_equal [$replica propagate-test.obeyed] 2} proc ::test) ``` We should wait for sync, otherwise the replica may not be fully synchronized before checking. The test was introduced in #1582. Signed-off-by: Binbin <[email protected]>
There is a timing issue in the test: ``` *** [err]: module RM_Call of expired key propagation in tests/unit/moduleapi/propagate.tcl Expected '1' to be equal to '2' (context: type eval line 27 cmd {assert_equal [$replica propagate-test.obeyed] 2} proc ::test) ``` We should wait for sync, otherwise the replica may not be fully synchronized before checking. The test was introduced in #1582. Signed-off-by: Binbin <[email protected]>
This PR adds support for MustObeyClient Module API.
The purpose of this API is for Modules to handle commands / callbacks knowing whether commands are arriving from the primary client or AOF client and should never be rejected.
A use case is that Modules can have validation logic in command handlers which only should be executed on primary nodes. Replica nodes must obey commands replicated.