There are six patches attached.
Two of them supersede the patches in my earlier message subject: "PATCH:
high_perf: fixes for accept optimizing". Newer versions of both patches
from that message are included here.
Summaries in alphabetical order:
==> client.pm.patch <==
First, since EventLoop goes off and does other things, any PostLoopCallback
can signal "our" EventLoop to return. To ensure we wait the full time, we
must loop around until the end condition is truly satisfied.
Second, why was the PostLoopCallback set after finishing the wait? There
doesn't seem to be any negative effect to instead simply clear the
PostLoopCallback altogether.
==> nicer.logs.patch <==
Make log messages more useful under pollserver. Log the FD whenever
possible along with the pid.
==> pollserver.noalarm.on.connect.patch <==
First, don't set alarm if in connect mode because check_earlytalker is all
but guaranteed to take longer than 2 seconds.
Second, move changeover to cmd mode because otherwise connect handlers
are run twice. Start_conversation runs them, and they recurse down into
process_line.
It may also be a good idea to set this alarm to 8 seconds because the spf
plugin uses a blocking DNS resolver and can take 5 seconds to timeout.
(Where 8 means >5.) However, this patch does not do that.
==> qpsmtpd.lean.log.in.loop.patch <==
This patch reverts the call to log methods with calls to ::log in qpsmtpd.
This noticeably reduces the memory usage for me. Also, the accept_handler
is called so often, perhaps it should be optimized this way.
==> socket.cmdtoconnect.patch <==
Move PLCMap maintenance into close and simply call close from DESTROY.
This also ensures that DescriptorMap and PushBackSet are maintained in the
event of an unexpected close.
==> socket.nostarve.plc.patch <==
Avoid starving the less fortunate PLCs (and stalling innocent connections)
by calling all of the PLCs. Maintains semantics of the return as specified
in comment above SetPostLoopCallback.
Brian
First, since EventLoop goes off and does other things, any PostLoopCallback
can signal "our" EventLoop to return. To ensure we wait the full time, we
must loop around until the end condition is truly satisfied.
Second, why was the PostLoopCallback set after finishing the wait? There
doesn't seem to be any negative effect to instead simply clear the
PostLoopCallback altogether.
--- lib/Danga/Client.pm 2005-05-18 16:11:31.000000000 -0600
+++ lib/Danga/Client.pm 2005-05-20 01:22:46.000000000 -0600
@@ -51,15 +51,19 @@
if (!length($self->{line})) {
my $old = $self->watch_read();
$self->watch_read(1);
- $self->SetPostLoopCallback(sub { (length($self->{line}) ||
- (Time::HiRes::time > $end)) ? 0 : 1
});
- #warn("get_line PRE\n");
- $self->EventLoop();
- #warn("get_line POST\n");
+ # loop because any callback, not just ours, can make EventLoop return
+ while( !(length($self->{line}) || (Time::HiRes::time > $end)) ) {
+ $self->SetPostLoopCallback(sub { (length($self->{line}) ||
+ (Time::HiRes::time > $end)) ? 0 :
1 });
+ #warn("get_line PRE\n");
+ $self->EventLoop();
+ #warn("get_line POST\n");
+ }
$self->watch_read($old);
}
$self->{can_read_mode} = 0;
- $self->SetPostLoopCallback(sub { $self->have_line ? 0 : 1 });
+ $self->SetPostLoopCallback(undef);
return if $self->{closing};
$self->{alive_time} = time;
# warn("can_read returning for '$self->{line}'\n");
Make log messages more useful under pollserver. Log the FD whenever
possible along with the pid.
--- lib/Qpsmtpd/PollServer.pm 2005-05-18 16:11:31.000000000 -0600
+++ lib/Qpsmtpd/PollServer.pm 2005-05-20 01:35:02.000000000 -0600
@@ -114,13 +145,14 @@
my $line = shift;
if ($self->{mode} eq 'connect') {
- warn("Connection incoming\n");
+ warn("$$:$self->{fd} Connection incoming\n");
my $rc = $self->start_conversation;
if ($rc != DONE) {
$self->close;
return;
}
$self->{mode} = 'cmd';
}
elsif ($self->{mode} eq 'cmd') {
$line =~ s/\r?\n//;
@@ -358,5 +397,15 @@
return;
}
+sub log {
+ my ($self, $trace, @log) = @_;
+ my $level = 4;
+ my $fd = $self->{fd};
+ $fd ||= '?';
+ warn join(" ", $$.':'.$fd, @log), "\n"
+ if $trace <= $level;
+}
+
+
1;
--- ../../../upstream/branches/high_perf/qpsmtpd 2005-05-18
16:11:31.000000000 -0600
+++ qpsmtpd 2005-05-18 21:08:44.000000000 -0600
@@ -426,7 +474,7 @@
sub log {
my ($level,$message) = @_;
# $level not used yet. this is reimplemented from elsewhere anyway
- warn("$$ $message\n");
+ warn("$$:? $message\n");
}
sub pause {
--- lib/Qpsmtpd.pm 2005-05-18 16:11:31.000000000 -0600
+++ lib/Qpsmtpd.pm 2005-05-18 21:08:44.000000000 -0600
@@ -32,7 +32,7 @@
my ($self, $trace, @log) = @_;
my $level = TRACE_LEVEL();
$level = $self->init_logger unless defined $level;
- warn join(" ", $$, @log), "\n"
+ warn join(" ", $$.':?', @log), "\n"
if $trace <= $level;
}
--- lib/Danga/Socket.pm 2005-05-18 16:11:31.000000000 -0600
+++ lib/Danga/Socket.pm 2005-05-20 01:14:33.000000000 -0600
@@ -712,7 +717,7 @@
}
elsif ($HaveEpoll) {
epoll_ctl($Epoll, EPOLL_CTL_MOD, $self->{fd}, $event)
- and print STDERR "couldn't modify epoll settings for
$self->{fd} " .
+ and print STDERR "$$:$self->{fd} couldn't modify epoll
settings for $self->{fd} " .
"($self) from $self->{event_watch} -> $event\n";
}
$self->{event_watch} = $event;
@@ -739,7 +744,7 @@
}
elsif ($HaveEpoll) {
epoll_ctl($Epoll, EPOLL_CTL_MOD, $self->{fd}, $event)
- and print STDERR "couldn't modify epoll settings for
$self->{fd} " .
+ and print STDERR "$$:$self->{fd} couldn't modify epoll
settings for $self->{fd} " .
"($self) from $self->{event_watch} -> $event\n";
}
$self->{event_watch} = $event;
First, don't set alarm if in connect mode because check_earlytalker is all
but guaranteed to take longer than 2 seconds.
Second, move changeover to cmd mode because otherwise connect handlers
are run twice. Start_conversation runs them, and they recurse down into
process_line.
It may also be a good idea to set this alarm to 8 seconds because the spf
plugin uses a blocking DNS resolver and can take 5 seconds to timeout.
(Where 8 means >5.)
--- lib/Qpsmtpd/PollServer.pm 2005-05-18 16:11:31.000000000 -0600
+++ lib/Qpsmtpd/PollServer.pm 2005-05-20 01:35:02.000000000 -0600
@@ -97,14 +122,20 @@
my ($pkg, $file, $line) = caller();
die "ALARM: ($self->{mode}) $pkg, $file, $line";
};
- my $prev = alarm(2); # must process a command in < 2 seconds
- eval { $self->_process_line($line) };
- alarm($prev);
- if ($@) {
- print STDERR "Error: [EMAIL PROTECTED]";
- return $self->fault("command failed unexpectedly") if $self->{mode} eq
'cmd';
- return $self->fault("error processing data lines") if $self->{mode} eq
'data';
- return $self->fault("unknown error");
+ if( $self->{mode} eq 'connect' ) {
+ eval { $self->_process_line($line) }
+ }
+ else {
+ my $prev = alarm(2); # must process a command in < 2 seconds
+ eval { $self->_process_line($line) };
+ alarm($prev);
+ if ($@) {
+ print STDERR "Error: [EMAIL PROTECTED]";
+ return $self->fault("command failed unexpectedly") if
$self->{mode} eq 'cmd';
+ return $self->fault("error processing data lines") if
$self->{mode} eq 'data';
+ return $self->fault("unknown error");
+ }
}
return;
}
@@ -114,13 +145,14 @@
my $line = shift;
if ($self->{mode} eq 'connect') {
warn("Connection incoming\n");
+ $self->{mode} = 'cmd';
my $rc = $self->start_conversation;
if ($rc != DONE) {
$self->close;
return;
}
- $self->{mode} = 'cmd';
}
elsif ($self->{mode} eq 'cmd') {
$line =~ s/\r?\n//;
This patch reverts the call to log methods with calls to ::log in qpsmtpd.
This noticeably reduces the memory usage for me. Also, the accept_handler
is called so often, perhaps it should be optimized this way.
--- qpsmtpd 2005-05-18 16:11:31.000000000 -0600
+++ qpsmtpd 2005-05-18 21:08:44.000000000 -0600
@@ -251,18 +287,21 @@
push @kids, spawn_child();
}
$SIG{INT} = $SIG{TERM} = sub { $SIG{CHLD} = "IGNORE"; kill 2 => @kids;
exit };
- $plugin_loader->log(LOGDEBUG, "Listening on $PORT with $PROCS children
$POLL");
+ ::log(LOGDEBUG, "Listening on $PORT with $PROCS children $POLL");
sleep while (1);
}
else {
if ($LineMode) {
$SIG{INT} = $SIG{TERM} = \&HUNTSMAN;
}
- $plugin_loader->log(LOGDEBUG, "Listening on $PORT with single process
$POLL" .
+ ::log(LOGDEBUG, "Listening on $PORT with single process $POLL" .
($LineMode ? " (forking server)" : ""));
Qpsmtpd::PollServer->OtherFds(fileno($SERVER) => \&accept_handler,
fileno($CONFIG_SERVER) =>
\&config_handler,
);
while (1) {
Qpsmtpd::PollServer->EventLoop();
}
@@ -354,13 +402,13 @@
}
if ($num_conn > $MAXCONNIP) {
- $client->log(LOGINFO,"Too many connections from $rem_ip: "
+ ::log(LOGINFO,"Too many connections from $rem_ip: "
."$num_conn > $MAXCONNIP. Denying connection.");
$client->write("451 Sorry, too many connections from $rem_ip,
try again later\r\n");
$client->close;
return 1;
}
- $client->log(LOGINFO, "accepted connection $running/$MAXCONN
($num_conn/$MAXCONNIP) from $rem_ip");
+ ::log(LOGINFO, "accepted connection $running/$MAXCONN
($num_conn/$MAXCONNIP) from $rem_ip");
}
$client->push_back_read("Connect\n");
@@ -414,7 +462,7 @@
$client->watch_read(1);
}
- $client->log(LOGDEBUG, "Finished with child %d.\n", fileno($csock))
+ ::log(LOGDEBUG, "Finished with child %d.\n", fileno($csock))
if $DEBUG;
$client->close();
Move PLCMap maintenance into close and simply call close from DESTROY.
This also ensures that DescriptorMap and PushBackSet are maintained in the
event of an unexpected close.
--- lib/Danga/Socket.pm 2005-05-18 16:11:31.000000000 -0600
+++ lib/Danga/Socket.pm 2005-05-20 01:14:33.000000000 -0600
@@ -358,6 +358,7 @@
while ($loop) {
$loop = 0;
foreach my $fd (keys %PushBackSet) {
+ next if ! $PushBackSet{$fd};
my Danga::Socket $pob = $PushBackSet{$fd};
next unless (! $pob->{closed} &&
$pob->{event_watch} & POLLIN);
@@ -480,6 +484,7 @@
}
}
+ delete $PLCMap{$fd};
delete $DescriptorMap{$fd};
delete $PushBackSet{$fd};
@@ -817,7 +822,8 @@
sub DESTROY {
my Danga::Socket $self = shift;
- delete $PLCMap{$self->{fd}};
+ $self->close() if !$self->{closed};
}
#####################################################################
Avoid starving the less fortunate PLCs (and stalling innocent connections)
by calling all of the PLCs. Maintains semantics of the return as specified
in comment above SetPostLoopCallback.
--- lib/Danga/Socket.pm 2005-05-18 16:11:31.000000000 -0600
+++ lib/Danga/Socket.pm 2005-05-20 01:14:33.000000000 -0600
@@ -374,15 +375,16 @@
@ToClose = ();
# now we're at the very end, call per-connection callbacks if defined
+ my $ret = 1; # use $ret so's to not starve some FDs; return 0 if any PLCs
return 0
for my $plc (values %PLCMap) {
- return unless $plc->(\%DescriptorMap, \%OtherFds);
+ $ret &&= $plc->(\%DescriptorMap, \%OtherFds);
}
# now we're at the very end, call global callback if defined
if (defined $PostLoopCallback) {
- return $PostLoopCallback->(\%DescriptorMap, \%OtherFds);
+ $ret &&= $PostLoopCallback->(\%DescriptorMap, \%OtherFds);
}
- return 1;
+ return $ret;
}
