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

Rewritten in python without cache functionality #7

Closed
wants to merge 7 commits into from

Conversation

adrianlzt
Copy link
Contributor

I have addressed two different problems.

First is cache functionality sometimes get broken, and always answer with the cached respose.

Second one, only one connection is allowed simultaneously, and we see several times that pyclustercheck is blocked because a connected client who doesn't release the connection.
Twisted is an easy way to have threads.

@Oneiroi
Copy link
Owner

Oneiroi commented Dec 10, 2013

Superceeds #4 #6 will look to review against virtual cluster.

@adrianlzt
Copy link
Contributor Author

I have implemented threads because I have seen several times pyclustercheck stucked with one connection.

The problem is if the client (probe) make the 3-way handshake, but never sends the GET HTTP request neither FIN tcp packet.

netstat will shown the connection as ESTABLISHED, and clustercheck will hang untill that connection is get closed.

To reproduce:
iptables -I INPUT -p tcp --tcp-flags ALL PSH,ACK -s 192.168.157.3 -j DROP
iptables -I INPUT -p tcp --tcp-flags ALL RST,ACK -s 192.168.157.3 -j DROP

With that, data packets (GET HTTP request) and connection close (RST,ACK tcp packets) will be dropped. Only the 3-way handshake will be done.

@Oneiroi
Copy link
Owner

Oneiroi commented May 8, 2014

merging in your master into a new branch for review; I am presently running a benchmark of this, per included multi-mechanize scripts.

AMD FX-8350 4 cores 8 threads
32GB 1600Mhz
120GB Intel 330 SSD

benchmarks constant 503 condition, from 2 x 100 thread pools over 10 minutes

Original BaseHTTPServer implementation with caching: https://github.com/Oneiroi/clustercheck/tree/master/benchmark/clustercheck/results/results_2014.05.08_09.57.45

Highlights:

  • 282407 requests
  • 0.368s ave response time
  • 1.236 95pct response time
  • 63.368 max response time

Fairly consistent ramp up graphs, ~ 500 tps

Notes: 0% cpu load from mysqld whilst cached completely idle

Twisted initial review: https://github.com/Oneiroi/clustercheck/tree/twisted_adrianlzt_review/benchmark/clustercheck/results/results_2014.05.08_10.31.51_twisted

  • 270552 requests
  • 0.302s ave response time (-0.066)
  • 0.443 95pct response time (-0.793)
  • 0.688 Max response time (-62.368)

~480 tps
much more consistent graphs
no apparent lockup occurring
Notes: 30-40% mysqld cpu load

I'm passing this arround for further peer review; it's very promising.

Thank you for contributing here; apologies for the delay in response hopefully we can get this merged in earnest shortly.

@Oneiroi
Copy link
Owner

Oneiroi commented May 8, 2014

@Oneiroi
Copy link
Owner

Oneiroi commented May 8, 2014

todo:

  1. Verify ipv6 support prevent regression of Add ipv6 support, disabled by default #8
    i) http://twistedmatrix.com/trac/ticket/5085
    ii) http://twistedmatrix.com/trac/wiki/IPv6
  2. Implement caching (maybe)

Oneiroi added a commit that referenced this pull request May 8, 2014
…ions to specify ipv4 or ipv6 variable bind address for v4 todo: variable bind addres for v6
@Oneiroi
Copy link
Owner

Oneiroi commented May 8, 2014

Commit 0856957 adds flags -4 / --ipv4 && -6 / --ipv6

-4 / --ipv6 is assumed to be default 0.0.0.0 and -6 / --ipv6 defaults to False

@Nibbler999
Copy link

This ipv6 support requires a newer version of twisted than is generally available on server distros.

Traceback (most recent call last):
  File "test.py", line 87, in <module>
    reactor.listenTCP(int(options.port), server.Site(ServerStatus()), interface=bind)
  File "/usr/lib64/python2.6/site-packages/twisted/internet/posixbase.py", line 356, in listenTCP
    p.startListening()
  File "/usr/lib64/python2.6/site-packages/twisted/internet/tcp.py", line 858, in startListening
    raise CannotListenError, (self.interface, self.port, le)
twisted.internet.error.CannotListenError: Couldn't listen on :::8000: [Errno -9] Address family for hostname not supported.

@Oneiroi
Copy link
Owner

Oneiroi commented May 8, 2014

@Nibbler999 thanks for testing and feedback I'll ensure the functionality is preserved before any merge is completed; not wanting any regression here; it would help to know which distro you have tested this on if possible please.

@Nibbler999
Copy link

That was CentOS 6.5. It has twisted 8.2. I also tested on the RHEL 7.0 RC where it works as expected.

@Oneiroi
Copy link
Owner

Oneiroi commented May 8, 2014

ignore previous I'm performing a complete rebuild of vm to ensure clean

@Oneiroi
Copy link
Owner

Oneiroi commented May 8, 2014

@Nibbler999 Confirmed none functional on 6.5

[vagrant@localhost clustercheck]$ python clustercheck.py -6
Traceback (most recent call last):
  File "clustercheck.py", line 108, in <module>
    reactor.listenTCP(int(options.port), server.Site(ServerStatus()), interface=bind)
  File "/usr/lib64/python2.6/site-packages/twisted/internet/posixbase.py", line 356, in listenTCP
    p.startListening()
  File "/usr/lib64/python2.6/site-packages/twisted/internet/tcp.py", line 858, in startListening
    raise CannotListenError, (self.interface, self.port, le)
twisted.internet.error.CannotListenError: Couldn't listen on :::8000: [Errno -9] Address family for hostname not supported.

@adrianlzt I'm creating a permanent branch here: https://github.com/Oneiroi/clustercheck/tree/twisted to host the twisted alternative, until we can avoid regression of #8 however I can not merge into master and thus recommend inclusion in PXC packages which is a shame given the perfromance and stability proven by the benchmarks; hopefully this may change in the near future.

@Oneiroi
Copy link
Owner

Oneiroi commented May 8, 2014

Closing this issue please refer to https://github.com/Oneiroi/clustercheck/tree/twisted for twisted version; thanks again for this contribution it's awesome!

@Oneiroi Oneiroi closed this May 8, 2014
@adrianlzt
Copy link
Contributor Author

Happy to see that it could help.
Thanks!

@Oneiroi
Copy link
Owner

Oneiroi commented May 16, 2014

@adrianlzt @Nibbler999 you'll find a blog post here http://www.mysqlperformanceblog.com/2014/05/16/benchmark-simplehttpserver-vs-pyclustercheck-twisted-implementation/ discussing this issue, and noting your contributions; thank you both again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants