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

Server performance test #893

Merged
merged 1 commit into from
Dec 1, 2017
Merged

Conversation

bruntib
Copy link
Contributor

@bruntib bruntib commented Sep 9, 2017

Resolves #808

@whisperity whisperity added the test ☑️ Adding or refactoring tests label Sep 9, 2017
VERBOSE = False


def timeit(func):
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we have similar functionality somewhere? Wouldn't it be better to put that to a common place and use it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The name collision might be a bit annoying. This decorator does something else.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it would be better to use a slightly different name? (So go to definition works better witihin IDEs).

Copy link
Member

@dkrupp dkrupp left a comment

Choose a reason for hiding this comment

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

  • Add a short readme.md how to use this script.
  • make the job generation rate configurable

A later development proposal:
-Now the client, when sends a job zips the results again, and again. We should save the clients resources to be able to generate higher rate. Could you modify the script so that the client side zipping is not repeated?


def play(self):
for name, func, args in self._actions:
self._user_random_sleep()
Copy link
Member

Choose a reason for hiding this comment

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

This random sleep time should follow exponential distribution and its mean should be configurable from command line.
"0" should mean that we do not wait anything (0s no random wait) between generating events.

print('-' * (40 + len('stdout')))


def parse_arguments():
Copy link
Member

Choose a reason for hiding this comment

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

Add a new argument -t which specifies the duration of the simulation and measurement. If not specified the test should run indefinitely long until CTRL-C is pressed. When exiting, the script should print the statistics.

@whisperity
Copy link
Contributor

I have a single concern: this script is too much of a tool which allows arbitrary users to attack and possibly overflow an arbitrary CodeChecker server. Shouldn't we somehow harden a live production deployment against such an attack?

My real concern is filling the disks beyond repair, assuming the server itself withstands the load.

@Xazax-hun
Copy link
Contributor

I think by default we do not accept any request that is not coming from the localhost. If you make your server public, authentication can protect you from such attacks. What kind of hardening can you think of? Database size quota?

@@ -0,0 +1,346 @@
import argparse
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add license info.



def parse_arguments():
parser = argparse.ArgumentParser(
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be useful to include default values of the argument in the help. Use argparse.ArgumentDefaultsHelpFormatter for this:

formatter_class=argparse.ArgumentDefaultsHelpFormatter,

type=int,
default=1,
help="Number of users")
parser.add_argument('-t', '--time',
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be better to use --timeout instead of --time for full argument name? You wrote in the help that this argument is a timeout.

metavar='PRODUCT_URL',
dest='product_url',
default='localhost:8001/Default',
required=True,
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be false because you specified a default value for this argument.

"of seconds. The random numbers have exponential "
"distribution of the beta parameter can be "
"provided here.")
parser.add_argument('-v', '--verbose',
Copy link
Contributor

Choose a reason for hiding this comment

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

I started to run this script without a running server. The output of this script was the following:

User 1 is sleeping 0.391030774259 seconds
Storage of performance_test_1 is started (~/.codechecker/tinyxml/)
Storage of performance_test_1 is done

This tells me that the storage was successful.

When I run this test script with the verbose flag the output says the storage was unsuccessful.

User 1 is sleeping 5.28858773684 seconds
Storage of performance_test_1 is started (/home/ecsomar/.codechecker/tinyxml-resolved/)
Output of storage
--------------------stdout--------------------
[09:39] - Storing analysis results for run 'performance_test_1'
Connection refused
[Errno 111] Connection refused

--------------------stderr--------------------

----------------------------------------------
Storage of performance_test_1 is done

Wouldn't it be better to write a better message to the user that the storage was successful or unsuccessful?

"""
self._stats[user_id].append((task_name, duration))

def print_stats(self, file_name):
Copy link
Contributor

Choose a reason for hiding this comment

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

The output of this function looks like this:

User,Storage,Comparison,Reports,Delete,Storage,Comparison                        
1,0.110867,0.12792,0.126015,0.090653,0.093675,0.113359

For my eyes it would be better to write this statistics in the following format:

---==== Statistics ====---
--------------------------
Action     | Time (ms)
--------------------------
Storage    | 0.110867
Comparison | 0.12792
Reports    | 0.126015
Delete     | 0.090653
Storage    | 0.093675
Comparison | 0.113359
--------------------------
----==================----
Number of users: 2
----==================----

We use this at the CodeChecker parse command when we write the statistics to the output.

@bruntib bruntib force-pushed the performance_test branch 2 times, most recently from 33c0bc9 to 92ca3e1 Compare November 21, 2017 17:26
@csordasmarton csordasmarton merged commit ac33873 into Ericsson:master Dec 1, 2017
@bruntib bruntib deleted the performance_test branch December 1, 2017 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test ☑️ Adding or refactoring tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants