Just a reminder: are there any more suggestions/remarks
before this feature can be applied?
Eduard.
On 26.03.2017 00:04, Eduard Bagdasaryan wrote:
Adjusted the patch accordingly.
Eduard.
On 22.03.2017 15:58, Amos Jeffries wrote:
On 17/03/2017 2:11 a.m., Eduard Bagdasaryan wrote:
On 16.03.2017 10:15, Amos Jeffries wrote:
in src/neighbours.cc:
* peerConnectTimeout() should be a member of the CachePeer class yes?
The initial patch version did as you suggest, but then we decided
to use separate method instead, to avoid 'heavy' dependency on
SquidConfig.h (which periodically causes linkage problems, IIRC).
Hmm. Okay for now, but please add a TODO or XXX about that to the
function.
in src/tunnel.cc:
* "start" is an action name and we use it (almost?) exclusively for
Job
initiation. By comparison "started" means/implies more clearly a state
or time point. The Tunnel member stores a time point.
- IMO both are bad, but "started" is better than "start". Better
still
would be a name that actually describes *what* has been start(ed).
I do not mind renaming it, e.g., into 'creationTime'.
Sounds good to me.
in src/PeerPoolMgr.cc:
* in PeerPoolMgr::handleOpenedConnection() I see a line doing max(1,
(peerTimeout - timeUsed)); just like the code in FwdState.cc
FwdState::connectDone().
BUT, this PoolMgr is (wrongly) using int instead of time_t. Perhapse
that should be changed and something de-duplicate that max(1, x)
usage?
I am not against changing int to time_t there. As for 'max', how about
creating inside neighbors.cc a helper method:
time_t PositiveTimeout(const time_t timeout) { return max(1,
timeout); }
Sounds good. I did a few tests to see if we could also avoid the
static_cast, but it seems not in any simple way. So dont forget that in
the new function.
in src/comm/Connection.cc:
* I dont see why EnoughTimeToReForward() should be in the Comm::
namespace. It is about messageing layer timing. So is more appropriate
to be in FwdState:: or maybe SquidTime.h/time.cc.
- if you pick time.cc that would also mean the fairly generic time_t
static functions it calls would move to time.cc where they seem more
appropriate.
Again, I am not against moving this method and ForwardTimeout() into
FwdState.cc (keeping related '*Forward' methods there makes some
sense).
Okay, I can live with that.
Amos
_______________________________________________
squid-dev mailing list
[email protected]
http://lists.squid-cache.org/listinfo/squid-dev
_______________________________________________
squid-dev mailing list
[email protected]
http://lists.squid-cache.org/listinfo/squid-dev
