-
Notifications
You must be signed in to change notification settings - Fork 53
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
Java: Changed handling of large requests to transfer them as leaked pointers #1696
Conversation
Java: Handle panics and errors in the Java FFI layer
Tests to be added. |
@jduo - is it something that could be applicable to a Python client as well? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please run spotless
(java linter) once you finish coding and rust linters (fmt
, clippy
and doc
)
*/ | ||
private static void populateRequestWithArgs(List<byte[]> arguments, Command.Builder outputBuilder) { | ||
final long totalArgSize = arguments.stream().mapToLong(arg -> arg.length).sum(); | ||
if (totalArgSize < RedisValueResolver.MAX_REQUEST_ARGS_LENGTH) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we count total args amount in a transaction?
Let say MAX_REQUEST_ARGS_LENGTH
is 100 and transaction has 20 commands with 20 args each. In that case request will be shipped with 20 arrays of args instead of 20 pointers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the concept of a transaction exist for Python? The Python PR didn't have this functionality. I'm inclined to treat that as a separate-but-related feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No idea unfortunately
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Transactions do exist for Python. I created #1713 to track this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, to be clear, it's not the total number of arguments in a command that is used to determine if a pointer is used. It's the total size in bytes of the arguments (that's why in the test case, we have two arguments to set() but they are 33MB each).
* Python: add XREVRANGE command * Update doc for xrevrange Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> * Update transaction docs Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> --------- Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> Co-authored-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
* Java: Add `FUNCTION DUMP` and `FUNCTION RESTORE`. (#370) * Add `FUNCTION DUMP` and `FUNCTION RESTORE` implementations. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Address PR comments. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Add tests. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Address PR comments. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> --------- Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Use GlideString Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> * Clean up FUNCTION DUMP & RESTORE Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> * Update handlers Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> * Update comments Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> * Add cluster IT test Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> * quick review comment Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> * SPOTLESS Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> --------- Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> Co-authored-by: Yury-Fridlyand <yury.fridlyand@improving.com>
support bitcount with GlideString
support bitop, bitpos and xdel with GlideString
valkey-io#1630) support hincrby, hincrbyfloat, incrby and incrbyfloat with GlideString
support llen, strlen, xlen and hstrlen with GlideString
…lkey-io#1658) * Python: add XGROUP CREATECONSUMER and XGROUP DELCONSUMER commands * Rename consumer variable to consumer_name
* Python: Add LOLWUT command (#387) Added Python LOLWUT command * Updated CHANGELOG.md * Fixed formated issues --------- Co-authored-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Java: Fix flaky IT. (#390) Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>
valkey-io#1642) * support objectEncoding, objectFreq, objectIdletime and objectRefcount with GlideString * add to integration tests the use of the API with GlideString parameters * add binary version integration tests * nit: spotlessApply * fix use of GlideString in objectRefcount_returns_null --------- Co-authored-by: Ubuntu <ubuntu@ip-172-31-152-44.ec2.internal>
…Time, pexpireTime, exists (valkey-io#1637) * GS version of ttl, pttl, expire, pexpire, expireAt, pexpireAt, expireTime, pexpireTime, exists Co-authored-by: Ubuntu <ubuntu@ip-172-31-19-27.ec2.internal>
…1635) * support geodist, getbit, setbit and xack with GlideString * add to integration tests the use of the API with GlideString parameters * nit: apply spotlessApply * remove calls to getBytes()
…lkey-io#1638) * support move, ltrim, sadd, srem and smove with GlideString * add to integration tests the use of the API with GlideString parameters * remove calls to getBytes() * nit: apply spotlessApply * add binary and not binary integration test for smove * add integration tests with GlideString version * nit: spotlessApply * remove the use of gs() in ltrim_existing_non_existing_key_and_type_error * add binary version integration tests
* Java: Add `LCS` command (with IDX option) (#386) * Implemented LCS with IDX * TODO: add docs and more integTests * Added docs and remaining tests * Addressed comments * Fixed rust formatting * Addressed comments * Added WITHMATCHLEN apis * Expanded on example * Fixed rust ci failure * Removed LcsOptions * Improved examples in docs * Examples with different match lengths * Throw NPE if matches is not present * Resolved conflicts
* Python: add XREADGROUP command * Fix mypy error * PR suggestions
* Java: Add XPENDING command (#389) * Java: Add XPENDING command Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> * Add @see doc Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> * Add @see doc Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> * Update constant name Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> * Make tests more robust Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> * Add UT tests Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> * Update docs for comments Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> --------- Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> * Update for review coments Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> * Fix merge conflicts Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> * Remove file Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> * Update options to point to valkey Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> --------- Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Closing this PR for now to create a PR tracking against bitquill/java/integ_lotjonat_panics |
Issue #, if available:
N/A
Description of changes:
Large protobuf messages are extremely slow to decode/encode, and with large messages (e.g., 512MB), the client becomes unresponsive. We resolve this issue by passing the arguments as a leaked pointer instead of a bytes vector when the message size exceeds the limit.
This PR is a port of #1655 for Java
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.