Re: [PATCH] sslcrtvalidator_children concurrency option default value

2013-12-11 Thread Tsantilas Christos
On 12/11/2013 12:09 AM, Amos Jeffries wrote:
 On 2013-12-11 10:46, Tsantilas Christos wrote:
 Hi all,

 currently we have the following situation for sslcrtvalidator_children
 configuration option, which is may confusing people:
  1) The testing sslcrtvalidator helper supports concurrency
  2) The default concurrency if the sslcrtvalidator_children is not set,
 is concurrency=0
  3) The default sslcrtvalidator_children line includes concurrency=1
  4) The documentation says:
  Defaults to 0 which indicates the certficate validator
   is a old-style single threaded redirector.

 This is make a confusion.

 I am attaching a simple patch which set the concurrency option for
 sslcrtvalidator_children by default to 1. I believe that this is the
 best because the testing helper we provide supports concurrency by
 default.

 Other options is
a) set sslcrtvalidator_children default line to use concurrency=0
b) Make a better documentation of the current behaviour to cf.data.pre


 Opinions?
 
 +1. Looks good. But can you change the new documentation line to avoid
 calling it a redirector and single-threaded?  A value of 0 indicates
 the certficate validator does not support concurrency. should be fine.

Thank you. I will apply with this change.


 
 Amos
 



[PATCH] sslcrtvalidator_children concurrency option default value

2013-12-10 Thread Tsantilas Christos
Hi all,

currently we have the following situation for sslcrtvalidator_children
configuration option, which is may confusing people:
 1) The testing sslcrtvalidator helper supports concurrency
 2) The default concurrency if the sslcrtvalidator_children is not set,
is concurrency=0
 3) The default sslcrtvalidator_children line includes concurrency=1
 4) The documentation says:
 Defaults to 0 which indicates the certficate validator
  is a old-style single threaded redirector.

This is make a confusion.

I am attaching a simple patch which set the concurrency option for
sslcrtvalidator_children by default to 1. I believe that this is the
best because the testing helper we provide supports concurrency by default.

Other options is
   a) set sslcrtvalidator_children default line to use concurrency=0
   b) Make a better documentation of the current behaviour to cf.data.pre


Opinions?
=== modified file 'src/cf.data.pre'
--- src/cf.data.pre	2013-12-05 11:04:45 +
+++ src/cf.data.pre	2013-12-10 20:02:47 +
@@ -2655,42 +2655,42 @@
 	
 		startup=N
 	
 	Sets the minimum number of processes to spawn when Squid
 	starts or reconfigures. When set to zero the first request will
 	cause spawning of the first child process to handle it.
 	
 	Starting too few children temporary slows Squid under load while it
 	tries to spawn enough additional processes to cope with traffic.
 	
 		idle=N
 	
 	Sets a minimum of how many processes Squid is to try and keep available
 	at all times. When traffic begins to rise above what the existing
 	processes can handle this many more will be spawned up to the maximum
 	configured. A minimum setting of 1 is required.
 
 		concurrency=
 	
 	The number of requests each certificate validator helper can handle in
-	parallel. Defaults to 0 which indicates the certficate validator
-	is a old-style single threaded redirector.
+	parallel. A value of 0 indicates the certficate validator is an 
+	old-style single threaded redirector. Defaults to 1.
 	
 	When this directive is set to a value = 1 then the protocol
 	used to communicate with the helper is modified to include
 	a request ID in front of the request/response. The request
 	ID from the request must be echoed back with the response
 	to that request.
 	
 	You must have at least one ssl_crt_validator process.
 DOC_END
 
 COMMENT_START
  OPTIONS WHICH AFFECT THE NEIGHBOR SELECTION ALGORITHM
  -
 COMMENT_END
 
 NAME: cache_peer
 TYPE: peer
 DEFAULT: none
 LOC: Config.peers
 DOC_START

=== modified file 'src/ssl/Config.cc'
--- src/ssl/Config.cc	2013-02-03 10:45:53 +
+++ src/ssl/Config.cc	2013-12-10 19:59:30 +
@@ -1,12 +1,21 @@
 #include squid.h
 #include ssl/Config.h
 
 Ssl::Config Ssl::TheConfig;
 
+Ssl::Config::Config():
+#if USE_SSL_CRTD
+ssl_crtd(NULL),
+#endif
+ssl_crt_validator(NULL)
+{ 
+ssl_crt_validator_Children.concurrency = 1;
+}
+
 Ssl::Config::~Config()
 {
 #if USE_SSL_CRTD
 xfree(ssl_crtd);
 #endif
 xfree(ssl_crt_validator);
 }

=== modified file 'src/ssl/Config.h'
--- src/ssl/Config.h	2013-02-03 10:45:53 +
+++ src/ssl/Config.h	2013-12-10 19:58:13 +
@@ -1,34 +1,29 @@
 #ifndef SQUID_SSL_CONFIG_H
 #define SQUID_SSL_CONFIG_H
 
 #include HelperChildConfig.h
 
 namespace Ssl
 {
 
 class Config
 {
 public:
 #if USE_SSL_CRTD
 char *ssl_crtd; /// Name of external ssl_crtd application.
 /// The number of processes spawn for ssl_crtd.
 HelperChildConfig ssl_crtdChildren;
 #endif
 char *ssl_crt_validator;
 HelperChildConfig ssl_crt_validator_Children;
-Config():
-#if USE_SSL_CRTD
-ssl_crtd(NULL),
-#endif
-ssl_crt_validator(NULL) {}
-
+Config();
 ~Config();
 private:
 Config(const Config ); // not implemented
 Config operator =(const Config ); // not implemented
 };
 
 extern Config TheConfig;
 
 } // namespace Ssl
 #endif



Re: [PATCH] sslcrtvalidator_children concurrency option default value

2013-12-10 Thread Amos Jeffries

On 2013-12-11 10:46, Tsantilas Christos wrote:

Hi all,

currently we have the following situation for sslcrtvalidator_children
configuration option, which is may confusing people:
 1) The testing sslcrtvalidator helper supports concurrency
 2) The default concurrency if the sslcrtvalidator_children is not set,
is concurrency=0
 3) The default sslcrtvalidator_children line includes concurrency=1
 4) The documentation says:
 Defaults to 0 which indicates the certficate validator
  is a old-style single threaded redirector.

This is make a confusion.

I am attaching a simple patch which set the concurrency option for
sslcrtvalidator_children by default to 1. I believe that this is the
best because the testing helper we provide supports concurrency by 
default.


Other options is
   a) set sslcrtvalidator_children default line to use concurrency=0
   b) Make a better documentation of the current behaviour to 
cf.data.pre



Opinions?


+1. Looks good. But can you change the new documentation line to avoid 
calling it a redirector and single-threaded?  A value of 0 indicates 
the certficate validator does not support concurrency. should be fine.


Amos