RFC: KIO::Scheduler changes... [PATCH included]

[Dawit A.]

This is a follow up on the the recent discussions about what KIO::Scheduler 
does by default and how we can go about improving the current default behavior 
to resolve some long standing issues in KDE as well as limit the number new 
ioslaves (read: new connections) that get spawned. For further details please 
read the the threads linked below or the explanation at the bottom of this 
email.

The patches provided here are my attempt to address the current short comings 
of the scheduler and are based on a patch David posted in [1] with the 
following additions/modifications:

1.) Add support for optional host based limits to the .protocol files 
(maxInstancesPerHost) and if set use them in KIO::Scheduler::scheduleJob.

2.) Modify the job.cpp class to ensure all redirected requests are re-
scheduled.

3.) Remove scheduled jobs if they are to be used in connection-oriented 
scheduling mode, KIO::Scheduler::assignJobToSlave.

---------------------------------------------------------------------------------------------------------------------------------------------

What follows is a brief explanation of how jobs are currently handled by the 
scheduler and how they will be handled when the attached patches are applied 
for those that do not want to be bothered to read the other threads linked 
below. 

First, for the benefit of those not familiar with the inner workings of KIO, 
here is roughly how a job is handled by KIO internally:

1.) A call to KIO::get, KIO::post, KIO::mimetype comes in...

2.) A job is created and of it is of type SimpleJob, it is handed over to 
KIO::Scheduler...

3.) KIO::Scheduler simply checks to see if there is an idle ioslave that can 
be reused to handle the request. If one is found, it is reused.

4.) If none was found, the scheduler will always attempt to create a brand new 
ioslave. No limits on the number of ioslaves it will create.

The issue with this is that the scheduler by default does not have any limits 
on the number ioslaves (read: new connections) that it spawns. Instead it 
forces you to call an additional function, KIO::Scheduler::scheduleJob, if you 
want to limit the number ioslave instances. 

In practise the problems with this default behavior only affects users under 
certain circumstances where servers actually put a limit on the number of 
connections they allow from a given address. This is specially true for more 
than a few ftp servers out there. In fact, there is a long standing FTP 
related bug vs Konqueror caused by this very issue. However, just because 
users do not directly see the side effects does not mean there is no problem 
with what the scheduler does by default. Creating too many connections by 
spawning many ioslaves is a costly affair to both to the client and server 
machines since it will unnecessarily result in the creation of many sockets. 
Hence, we end up using too many resources.

Okay so what does this attach patch do ?

1.) Schedule all simple jobs created through KIO::<get/post/..> by default.

2.) Add an optional "maxInstancesPerHost" property to the *.protocol files that 
is honored when jobs are scheduled which is now the default. KIO::scheduleJob 
currently honors the existing "maxInstances" property set in each *.protocol 
file as a 'soft limit'. However, if #1 is made the default, then this will 
cause a bottle neck at the scheduler since the "maxInstances" property is per 
protocol (ftp, http). For example, for the ioslaves included in kdelibs, these 
value is set to 4, 3, and 2 in file, http and ftp protocol files respectively. 
Having only this many instances of ioslaves KDE wide is not sufficient. Hence 
the new property. After that it is only a matter of setting both 
"maxInstances=" and "maxInstancesPerHost=" proerties to some sane default 
values so that the scheduler can do a better job of scheduling requests per 
host. 

For example, fot the http protocol the "maxInstances" property can be raised 
to "15", 5x as much as its original value, and "maxInstancesPerHost" set to 
"3". This means that there cannot be more than 15 instances of the http 
ioslave at any given time and no more than 3 connection to any given host. All 
jobs will be queued until this criteria is fulfilled...

What issues remain ?

1.) Unfortunately the patch to job.cpp will break applications that might want 
to have direct control over the number of ioslaves (read: connections) , e.g. 
a download manager. The change to scheduling jobs by default will make direct 
control impossible since all jobs will be queued. So, an additional function 
is required in KIO::Scheduler to remove the scheduled job from the queue or 
such applications need to call KIO::Scheduler::doJob again and we can add the 
remove from queue check there ??? Somehow the last one seems very inefficient to 
me, doJob(...), scheduleJob(...) and then doJob(...) again...

2.) Should the other scheduling modes, besides "scheduled mode" be forced to 
honor the "maxInstances" property ? My own view is no since the default does 
and the other modes are meant to give the users of KIO complete control to 
schedule their jobs how they see fit.

Anyhow, comments, suggestions, improvements and everything in between is 
welcome...

[1] http://lists.kde.org/?t=124531188100002&r=1&w=2
[2] http://lists.kde.org/?t=125042610900001&r=1&w=2
-------------- next part --------------
A non-text attachment was scrubbed...
Name: scheduler.patch
Type: text/x-patch
Size: 5584 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20090828/db6e6b02/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: job.patch
Type: text/x-patch
Size: 1290 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20090828/db6e6b02/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: kprotocolinfo.patch
Type: text/x-patch
Size: 3166 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20090828/db6e6b02/attachment-0002.bin>