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

ISPN-16076 Convert RadarGun test to Hyperfoil #19

Merged
merged 1 commit into from
Jun 10, 2024

Conversation

jabolina
Copy link
Member

This PR is the initial setup for the read-write test in RadarGun. We only need a single template file. We can then split the read/write ratio and create the actual benchmark based on those values. By default, it runs 80% reads and 20% writes.

Pending points:

  • The use_auth doesn't seem to propagate when running with the all_benchmarks playbook;
  • The load_multiplier value is still a placeholder until we find the actual limit in the server (https://issues.redhat.com/browse/ISPN-16077).

I am creating it as a draft until we have everything round.

@jabolina
Copy link
Member Author

To reproduce the issue in AWS, just run the benchmarks as is with this load and the default with auth enabled. The test runs for 4 mins. The generated report should end with a weird result:

image

And some errors complaining of CPU utilization.

@jabolina
Copy link
Member Author

I've been running the agent with -e agent_java_args="'-Xms128m -Xmx800m -XX:StartFlightRecording=delay=10s,duration=24h,settings=profile,filename=/tmp/hyperfoil/workspace/hfa-hotrod.jfr'".

I also tried disabling C2 and increasing the new generation size. Sometimes, it completes just fine, but sometimes, I still see the 100% CPU spike. There were cases in which just one of the clients showed this behavior. I am looking into the JFR, and I'll try to compare what happened.

@jabolina
Copy link
Member Author

I'll be updating this with our current findings. We'll utilize a max_active with a value of 2 in all Hot Rod tests.

@jabolina
Copy link
Member Author

I'll investigate until we have concrete baseline numbers with this new configuration. I'll keep the base t2.micro instances for this. We have tests for DIST and REPL with 3 server nodes.

test_name: "{{ benchmark.name }}"
arguments: "{{ benchmark.arguments | default('{}')}}"
Copy link
Member

Choose a reason for hiding this comment

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

I like this, cool reuse!

Comment on lines 1 to 7
- local_action:
module: copy
content: "{{ arguments | to_json }}"
dest: "/tmp/arguments.json"

- name: Load server arguments
ansible.builtin.include_vars: "/tmp/arguments.json"
Copy link
Member

Choose a reason for hiding this comment

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

Can this not be done with a loop invoking set_fact dynamically? Set fact can use dynamic variable arguments such as https://github.com/infinispan/infinispan-ansible-benchmark/blob/main/roles/aws_ec2/tasks/flat-map-instances.yml#L15

Copy link
Member Author

Choose a reason for hiding this comment

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

Hrrrmmm, let me see. The set_fact would need to accept a JSON variable as input.

Copy link
Member

@wburns wburns May 28, 2024

Choose a reason for hiding this comment

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

You don't need to. It is already in the ansible variable. You don't need the local_action above either. Just do a loop on the arguments variable which should be a dict at this point.

  ansible.builtin.set_fact:
    "{{ item.key }}" : "{{ item.value }}"
  loop: "{{ arguments | dict2items }}"

Can't remember if you need the dict2items, you might not at all

Copy link
Member Author

Choose a reason for hiding this comment

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

It almost works, but the set_fact seems to convert everything to string, which causes issues when parsing the templates.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hrrm, it seems there's an option for that: https://stackoverflow.com/a/62268930/9244031

Copy link
Member

Choose a reason for hiding this comment

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

I think the benchmark template can do the cast directly, no?

Copy link
Member

Choose a reason for hiding this comment

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

Btw we need to do something to fix it because I also can't pass -e read_ratio=0.8 -e write_ratio=0.2 to ansible-playbook without getting the same error.

Copy link
Member

@wburns wburns May 30, 2024

Choose a reason for hiding this comment

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

Yeah, I change benchmark to be write_ratio | float and the same with read_ratio in all the places I can pass a command line argument as well, which will also fix the cast issue you had. We should probably do that for any arguments that may be overridden in the benchmark file by the user.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's great. I've updated the benchmark adding a cast to all the numeric parameters.

Added benchmark template that changes dynamically with read-write ratio.
@wburns wburns merged commit 93ed512 into infinispan:main Jun 10, 2024
@wburns
Copy link
Member

wburns commented Jun 10, 2024

Integrated, thanks @jabolina !

@jabolina jabolina deleted the ISPN-16076 branch June 10, 2024 17:27
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.

None yet

2 participants