Bug 13120 - [CVS Repo View] Repository view should recycle recently opened connections
Summary: [CVS Repo View] Repository view should recycle recently opened connections
Status: RESOLVED INVALID
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Team (show other bugs)
Version: 2.0   Edit
Hardware: PC Windows 2000
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Kevin McGuire CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
Depends on:
Blocks:
 
Reported: 2002-04-09 17:41 EDT by Jeff Brown CLA
Modified: 2002-09-17 09:22 EDT (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jeff Brown CLA 2002-04-09 17:41:21 EDT
Expanding elements in the tree for the first time causes their
contents to be loaded from the respository.  Each time a node
is expanded, a new connection to the server is created, used,
then discarded resulting in a long turn-around time.  The
effect is particularly noticeable while drilling down several
levels into the tree on slow networks (particularly with the EXT
or EXTSSH protocols).
Comment 1 Kevin McGuire CLA 2002-04-10 14:19:51 EDT
So when would you know to close the connection?
Comment 2 Jeff Brown CLA 2002-04-11 13:50:32 EDT
I have been toying with the idea of using a connection pool of
some sort.  This could be hidden underneath the existing Session
open() and close() methods and hence would be fully transparent
to existing code.

So we'd have something like:
class ConnectionPool {
  public Connection acquireConnection(ICVSRepositoryLocation location,
    IProgressMonitor monitor) throws CVSException {
    // if there is an open connection in the pool for that location,
    //    remove it from the pool and return it
    // otherwise create a new connection, perform the initial negotiation
    //    phases, associate the progress monitor with the connection
    //    for cancellation, and return it
  }
  public void releaseConnection(Connection connection, boolean canReuse)
    throws CVSException {
    // if canReuse and the connection is not yet closed, add the connection
    //    to the pool and remember the time at which it was added
    //    (if an error occurred, this will be false)
    // otherwise close the connection
  }
  // background thread wakes up every few seconds, looks for connections
  // in the pool that should be expired, then closes them silently in
  // the background -- connections might expire after 90 seconds or so...
}

Tricky bits:
1) determining when we can recycle a connection (amounts to remembering if
   a fatal connection error occurred)
2) ensuring that i/o buffers are properly maintained across multiple uses
3) associating the new progress monitor with the streams for low-level
   cancellation
4) handling unexpected closure of a connection when obtaining a
   connection from the pool (simplest solution is to send a noop request
   to the server to verify that the connection is still alive before
   returning it and to fall back to opening a new connection if this fails)
5) handling cases where the definition of a repository location or
   certain critical CVS preferences (such as timeouts and compression level)
   have been changed in the UI since the connection was first created
Comment 3 Michael Valenta CLA 2002-04-11 14:44:29 EDT
There are a few other tricky bits to consider:
1. How long can a connection stay open?
2. How are we notified that we should close? (I assume it must be a separate 
thread which raises all sorts of syncronization issues).
3. How do we know when a request can reuse an existing open connection? 
Connections have state that is accumulated as commands are sent over the 
connection. As far as I know, it's only the directory structure.

Although there are advantages to this as a general mechanism, the possible 
complications make me feel that its not something we want look at for 2.0. 
However, there may be a few particular instances that we may want to try and 
improve that involve explicit opening and closing. This include refreshing the 
repository view (which could potentially have many folders open) and fetching 
the contents of remote files during a sync or merge. This may be important 
because it is possible that a server may confuse these situations with a DOS 
attack.
Comment 4 Jeff Brown CLA 2002-04-11 16:33:22 EDT
You are right, I see this as a post 2.0 item.  The main advantage
is that it does not require us to modify the RemoteFile interfaces
to support processing a series of requests in one go so the UI
code can still be connection pooling agnostic (may or may not be
a good thing...)

There are really two classes of problems here:

Cases with good temporal locality (long-running user requests):
- populating the merge editor
- refreshing the repository view

Cases with limited temporal locality (short and interactive user requests):
- browsing the repository view
- browsing changes in the sync view
Comment 5 Kevin McGuire CLA 2002-04-25 14:26:09 EDT
post 2.0
Comment 6 Michael Valenta CLA 2002-09-09 15:51:43 EDT
Reopening
Comment 7 Michael Valenta CLA 2002-09-17 09:22:57 EDT
This is no good as multiple commands over a single connection doesn't work with 
all server configurations