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.

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

[redis]: Use Readline to read large value #471

Merged
merged 2 commits into from
Dec 21, 2021

Conversation

marcaudefroy
Copy link
Collaborator

By default, Scanner uses a buffer with 64K capacity. So when we try to set a huge redis value (>64K), executor failed to read file. The fix consists to use Readline method. (source https://devmarkpro.com/working-big-files-golang).

Before :

 ./venom run ./../../tests/redis.yml
 • Redis testsuite (./../../tests/redis.yml)
 	• Commands_Test_Case SUCCESS
 	• File_Test_Case SUCCESS
 	• Long_Value_File_Test_Case FAILURE
Testcase "Long_Value_File_Test_Case", step #1: Failed to load file: bufio.Scanner: token too long (./../../tests/redis.yml:50)
 	• Commands_Nested_Array_Response_Test_Case SUCCESS
	  [info] streamEntry1ID: OK (./../../tests/redis.yml:70) (./../../tests/redis.yml:70)
	  [info] streamEntry2ID: 1639752707532-1 (./../../tests/redis.yml:72) (./../../tests/redis.yml:72)

After :

   ./venom  run -v ./../../tests/redis.yml
	  [trac] writing venom.log
 • Redis testsuite (./../../tests/redis.yml)
 	• Commands_Test_Case SUCCESS
 	• File_Test_Case SUCCESS
 	• Long_Value_File_Test_Case SUCCESS
 	• Commands_Nested_Array_Response_Test_Case SUCCESS
	  [info] streamEntry1ID: OK (./../../tests/redis.yml:70) (./../../tests/redis.yml:70)
	  [info] streamEntry2ID: 1639753471610-1 (./../../tests/redis.yml:72) (./../../tests/redis.yml:72)

@marcaudefroy marcaudefroy changed the title [redis]: Use File Stat to determine the size of scanner buffer [redis]: Use Readline to read large value Dec 20, 2021
@marcaudefroy marcaudefroy force-pushed the fix_redis_executor_when_large_value branch from cc4cdef to 9515e29 Compare December 20, 2021 09:36
@yesnault yesnault merged commit eeb7e4c into ovh:master Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants