Add python script for zookeeper benchmark #27

aayustark007-fk · 2023-06-27T08:18:22Z

This script connects to zookeeper ensemble, loads data under a parent node and then measures the response time of get_children() call.

Various parameters are configurable and can be fetched by running python zk_benchmark.py -h
To speed up data loading, there are two options, multi-threaded dataloader and multi-process dataloader. Multi-process dataloader has some bugs so its not enabled by default.

Requirements: python 3.10+, tqdm, kazoo

github-actions · 2023-06-27T08:23:36Z

Test Results

21 tests 21 ✔️ 11s ⏱️
  9 suites   0 💤
  9 files   0 ❌

Results for commit 1c48164.

♻️ This comment has been updated with latest results.

kmrdhruv · 2023-06-28T13:50:17Z

scripts/zk_benchmark.py

+ configs = self.__get_chunked_configs(child_node_config)
+ with concurrent.futures.ProcessPoolExecutor(max_workers=self.config.parallelism) as exec:
+ futures_to_id = {exec.submit(self._process, parent, config): id for (id, config) in enumerate(configs)}
+ for future in concurrent.futures.as_completed(futures_to_id):


want to have upper time bound to terminate it deterministically.

the API for multiprocess executor does not provide a way to pass timeout
trying to find a way to pass sigterm to subprocesses

may be kill process group pgid()

So, I read that by using with statement will cause executor.shutdown() to be called which will terminate the processes. My assumption is that if I send a SIGINT via ctrl+c it should cause program to terminate and the with block to get cleaned up.

But, while running I see that when zookeeper becomes unreachable, the multiprocess program behaves badly. The program gets stuck and upon SIGTERM the main process exits but some of the processes continue to exists as zombies. (sending logs to the terminal)

kmrdhruv · 2023-06-28T14:07:30Z

scripts/zk_benchmark.py

+ print("process: {} generated an exception: {}".format(id, exc))
+
+ def _process(self, parent: str, child_node_config: ChildNodeConfig):
+ with DataLoader(self.config, zk_config=self.zk_config) as loader:


is this creating threadpool in processpool, expected ?

Yes, this is a wrapper over thread pool data loader and we ensure that the chunks are divided such that the wrapped dataloader only works on a single chunk.

The reason it is written this way is because, I initially wrote the multithreaded dataloader but the performance was not sufficient due to the limitations imposed by the GIL. Then, I wrote the multi process implementation which tries to share the current object state between the processes. This approach failed due to the requirement that the arguments (and hence the object state) should be pickleable and the zookeeper client was not pickleable. Therefore, I wrapped over existing Dataloader instance which separately creates a zookeeper client in the new process instead of sharing it.

I'm thinking of writing a single threaded data loader, then the multiprocess data loader can wrap over it.

kmrdhruv · 2023-06-28T14:08:29Z

scripts/zk_benchmark.py

+ def __exit__(self, exc_type, exc_val, exc_tb):
+ return True
+
+ def __get_chunked_configs(self, child_node_config: ChildNodeConfig) -> list[ChildNodeConfig]:


with default chunk_size of 10K, and target range of 10K, this will effectively generate single block ?
should default chunk_size be smaller number say 1K ?

yes, but the intention is that users of this script will benchmark for node count > 10k
if they need to create less then they can reduce the chunk size
but I think default chunk size can be reduced to 1k

kmrdhruv · 2023-06-28T14:15:50Z

scripts/zk_benchmark.py

+ print("loader: {} generated an exception: {}".format(id, exc))
+
+ def _fill(self, path: str, child_generator: NodeGenerator, gen_size: int):
+ for node in tqdm(child_generator, total=gen_size):


does tqdm will take care of parallel execution in terms of progress bar display for each chunk ?

yes it works well with multi threaded executor, but not with multiprocess executor where the progress bars start to overlap

kmrdhruv · 2023-06-28T14:36:34Z

scripts/zk_benchmark.py

+ while gen < num:
+ name_res = ""
+ if len(name) > 0:
+ name_res = "{}_{}".format(name, str(gen + 1))


this wouldn't take care of name length requirements, but that should be fine in case fixed name is needed. just add a comment for the same.

Actually the intention is that of name is given then min_len max_len will not be used. Will add comments describing the same.

kmrdhruv · 2023-06-28T14:45:18Z

scripts/zk_benchmark.py

+ with catchtime() as t:
+ for _ in range(measure_samples):
+ client.ls(path)
+ print("Latency get_children on path {}: {} ms".format(path, t.time / measure_samples))


in this case it wouldn't matter much, but it would be good to have it as min, max, avg over the sample range.

agree, I'll add this change

kmrdhruv · 2023-06-28T15:01:23Z

move this to scripts/benchmarks directory and also add a small readme under same folder on how to install dependencies and run the benchmark as well.

kmrdhruv

move this to scripts/benchmarks directory and also add a small readme under same folder on how to install dependencies and run the benchmark as well.

Signed-off-by: aayustark007-fk <[email protected]>

…ess loader impl Signed-off-by: aayustark007-fk <[email protected]>

Signed-off-by: aayustark007-fk <[email protected]>

kmrdhruv · 2023-07-03T11:00:07Z

scripts/benchmark/README.md

+### Currently, holds the script for benchmarking read performance of Zookeeper
+
+Requires: `Python==3.9.7+`
+


pip -r requirements.txt ? needed ?

kmrdhruv · 2023-07-04T17:01:11Z

scripts/benchmark/zk_benchmark.py

@@ -312,6 +315,7 @@ def __run(self, run_config: BenchmarkRunConfig, skip_measure: bool, dataloading_

 ## Measure step
 self.__measure(path, measure_samples, skip_measure)


nit: __measure_list()

kmrdhruv

LGTM.
Do execute the benchmark on a standard zk setup and publish results.

kmrdhruv reviewed Jun 28, 2023

View reviewed changes

kmrdhruv requested changes Jun 28, 2023

View reviewed changes

AayuStark007 added 3 commits June 30, 2023 14:33

add python script for zookeeper benchmark

95d204e

Signed-off-by: aayustark007-fk <[email protected]>

remove unused vars

cac50f1

Signed-off-by: aayustark007-fk <[email protected]>

address PR comments

c0e2ed9

Signed-off-by: aayustark007-fk <[email protected]>

aayustark007-fk force-pushed the zk_benchmark branch from 0cc7d88 to c0e2ed9 Compare June 30, 2023 11:49

AayuStark007 added 2 commits July 2, 2023 22:30

refactor code to use single threaded dataloader wrapped by multi proc…

8b2d153

…ess loader impl Signed-off-by: aayustark007-fk <[email protected]>

add usage examples to readme

e4a915a

Signed-off-by: aayustark007-fk <[email protected]>

kmrdhruv reviewed Jul 3, 2023

View reviewed changes

add measure get data and readme updates

4fa9f68

kmrdhruv reviewed Jul 4, 2023

View reviewed changes

kmrdhruv approved these changes Jul 4, 2023

View reviewed changes

AayuStark007 added 2 commits July 5, 2023 14:25

rename __measure to __measure_list

1c48614

add flag to skip data loading and measure directly

1c48164

aayustark007-fk requested a review from kmrdhruv July 11, 2023 05:55

kmrdhruv approved these changes Jul 12, 2023

View reviewed changes

kmrdhruv merged commit 612dda2 into master Jul 13, 2023
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add python script for zookeeper benchmark #27

Add python script for zookeeper benchmark #27

aayustark007-fk commented Jun 27, 2023

github-actions bot commented Jun 27, 2023 •

edited

Loading

kmrdhruv Jun 28, 2023

aayustark007-fk Jun 30, 2023

kmrdhruv Jul 3, 2023

aayustark007-fk Jul 5, 2023 •

edited

Loading

kmrdhruv Jun 28, 2023

aayustark007-fk Jun 29, 2023

kmrdhruv Jun 28, 2023 •

edited

Loading

aayustark007-fk Jun 29, 2023

kmrdhruv Jun 28, 2023

aayustark007-fk Jun 29, 2023

kmrdhruv Jun 28, 2023

aayustark007-fk Jun 29, 2023

kmrdhruv Jun 28, 2023

aayustark007-fk Jun 29, 2023

kmrdhruv commented Jun 28, 2023

kmrdhruv left a comment

kmrdhruv Jul 3, 2023

kmrdhruv Jul 4, 2023

kmrdhruv left a comment

		### Currently, holds the script for benchmarking read performance of Zookeeper

		Requires: `Python==3.9.7+`

		@@ -312,6 +315,7 @@ def __run(self, run_config: BenchmarkRunConfig, skip_measure: bool, dataloading_

		## Measure step
		self.__measure(path, measure_samples, skip_measure)

Add python script for zookeeper benchmark #27

Add python script for zookeeper benchmark #27

Conversation

aayustark007-fk commented Jun 27, 2023

github-actions bot commented Jun 27, 2023 • edited Loading

Test Results

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aayustark007-fk Jul 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmrdhruv Jun 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmrdhruv commented Jun 28, 2023

kmrdhruv left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmrdhruv left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 27, 2023 •

edited

Loading

aayustark007-fk Jul 5, 2023 •

edited

Loading

kmrdhruv Jun 28, 2023 •

edited

Loading