-
Notifications
You must be signed in to change notification settings - Fork 122
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
Enhance data caching client #399
Conversation
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.
I leave some comment on the document, and still working on testing the docker image.
docs/benchmarks/data-caching.md
Outdated
|
||
## Important remarks ## | ||
- It takes several minutes for the server to reach a stable state. | ||
|
||
- The target QoS requires that 95% of the requests are serviced within 10ms. | ||
- The target QoS requires that 95% of the requests are serviced within 10 ms. |
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.
I think the SLO for data caching should be 1ms. 10ms is roughly the time of reading from the disk, so it is not meaningful to caching in memory.
|
||
$ docker run -idt --name dc-client --net host -v PATH_TO_DOCKER_SERVERS_FOLDER:/usr/src/memcached/memcached_client/docker_servers/ cloudsuite/data-caching:client | ||
|
||
Please note that the command mounts the folder containing the 'docker_servers.txt' file instead of mounting only the file. This way, further changes to the docker_servers.txt file in the host will be reflected inside of the container. |
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.
Maybe directly mapping the docker_servers.txt
file into the container is more direct than mapping the folder. What do you think?
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.
I don't remember ever modifying anything besides the docker_servers.txt
file, so yes I think we should just map that one file.
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.
I recommend mapping a folder because this way, changes to the file in the host after starting the container will be reflected inside the container. Otherwise, if a user changed the file in the host, the inode of the file changes and the container will have a stale file.
|
||
To start the client container use the following command: | ||
|
||
$ docker run -idt --name dc-client --net host -v PATH_TO_DOCKER_SERVERS_FOLDER:/usr/src/memcached/memcached_client/docker_servers/ cloudsuite/data-caching:client |
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.
If the container is running as a daemon -d
, we don't have to make it interactive -it
. Just leaving -d
is enough.
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.
After trial, I think we don't need -i
, but -t
is important to keep the container alive.
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.
I recommend keeping -i
because a user may want to enter the container later and modify it manually for any reason.
docs/benchmarks/data-caching.md
Outdated
$ ./loader -a ../twitter_dataset/twitter_dataset_unscaled -o ../twitter_dataset/twitter_dataset_30x -s docker_servers.txt -w 4 -S 30 -D 4096 -j -T 1 | ||
|
||
(`w` - number of client threads which has to be divisible to the number of servers, `S` - scaling factor, `D` - target server memory, `T` - statistics interval, `s` - server configuration file, `j` - an indicator that the server should be warmed up). | ||
$ docker exec -it dc-client /bin/bash /entrypoint.sh --m="S&W" --S=30 --D=1024 --w=8 --T=1 |
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.
1024 to 10240
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.
Do we need exec /bin/bash /entrypoint.sh
, I though by default if we pick run
, it will do the equivalent to that command?
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.
I think we need it because we are using the exec
command and this command needs a binary to run. I am not using run
command because we have broken the client's tasks into multiple subtasks, and we call each separately using the exec
command.
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.
@Xusine1131 Actually the dataset with scale factor 30 is bigger than 10 GB. So maybe we recommend 11 GB instead of 10 GB in the document.
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.
The scaling factor is a function of the recommended dataset size, which should lead to a representative state for measurement.
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.
Once we fixed the document, we can merge this PR.
This PR removes the need for manipulating the client container by going inside it. The entrypoint has the required commands for different operation modes of the client container.