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

Error reported when running with "--data-import" #238

Closed
DarrenJiang13 opened this issue Oct 23, 2023 · 1 comment · Fixed by #240
Closed

Error reported when running with "--data-import" #238

DarrenJiang13 opened this issue Oct 23, 2023 · 1 comment · Fixed by #240

Comments

@DarrenJiang13
Copy link

Problem

Hi guys, when I run the newest master branch with "--data-import", an error happened:

memtier_benchmark: obj_gen.cpp:438: const char* object_generator::get_value(long long unsigned int, unsigned int*): Assertion `0' failed.

And I found this is brought by commit 1c6735f , before this commit "--data-import" worked OK.

Discussion

IMHO the problem should be in client::create_set_request(). I change it like this and things go right. But I am not sure this is the right solution
image
@YaacovHazan
Copy link
Collaborator

@DarrenJiang13 , thanks for reporting this bug.
You are right, the --data-import broke by commit 1c6735f, but unfortunately the fixed is not that simple.

I will work on a fix and update.

YaacovHazan added a commit to YaacovHazan/memtier_benchmark that referenced this issue Oct 31, 2023
@YaacovHazan
see RedisLabs#238, data import broken
31caa28 
The PR includes some refactoring and cleanup around the following:
object_generator
    - generate_key(), a new function that generates a key and stores it
      inside internal buffer (no need for a buffer at client level)
import_object_generator
    - Align it to object_generator APIs of getting key/value/expire
    - read_next_item - new function for reading the next item before
      creating SET command
    - read_next_key - new function for reading (pointing) to the next
      key before creating the GET command
data_object
    - removed.
      We were already getting key/value/expire separately. Now that
      data-import and verify-data also moved to do it like that,
      we could remove it completely.
verify_client
    - unify the craate_request with its base class, and derive the
      create_x_request functions.
@filipecosta90 filipecosta90 linked a pull request Nov 16, 2023 that will close this issue
see #238, data import broken #240
Merged
filipecosta90 added a commit that referenced this issue Dec 2, 2023
@YaacovHazan @filipecosta90
see #238, data import broken (#240)
e08d4cf 
* see #238, data import broken

The PR includes some refactoring and cleanup around the following:
object_generator
    - generate_key(), a new function that generates a key and stores it
      inside internal buffer (no need for a buffer at client level)
import_object_generator
    - Align it to object_generator APIs of getting key/value/expire
    - read_next_item - new function for reading the next item before
      creating SET command
    - read_next_key - new function for reading (pointing) to the next
      key before creating the GET command
data_object
    - removed.
      We were already getting key/value/expire separately. Now that
      data-import and verify-data also moved to do it like that,
      we could remove it completely.
verify_client
    - unify the craate_request with its base class, and derive the
      create_x_request functions.

* Included data import test cases

* Cluster mode cannot be specified when importing

* Fixed merge of tests from master

---------

Co-authored-by: YaacovHazan <yaacov.hazan@redis.com>
Co-authored-by: filipecosta90 <filipecosta.90@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

see #238, data import broken YaacovHazan/memtier_benchmark
2 participants
@DarrenJiang13 @YaacovHazan