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

Enhance data caching client #399

Merged
merged 4 commits into from
Feb 13, 2023
Merged

Enhance data caching client #399

merged 4 commits into from
Feb 13, 2023

Conversation

aansaarii
Copy link
Contributor

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.

@aansaarii aansaarii requested a review from xusine January 30, 2023 18:19
Copy link
Contributor

@xusine xusine left a 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.


## 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.
Copy link
Contributor

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.
Copy link
Contributor

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?

Copy link
Collaborator

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.

Copy link
Contributor Author

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
Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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.

$ ./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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1024 to 10240

Copy link
Collaborator

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor

@xusine xusine left a 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.

@UlisesLuzius UlisesLuzius mentioned this pull request Feb 9, 2023
21 tasks
@xusine xusine merged commit f753465 into main Feb 13, 2023
@UlisesLuzius UlisesLuzius deleted the enhance_data_caching_client branch February 13, 2023 16:42
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.

3 participants