oblac / jodd

Jodd! Lightweight. Java. Zero dependencies. Use what you like.
https://jodd.org
BSD 2-Clause "Simplified" License
4.06k stars 723 forks source link

jodd works not well with tomcat & mysql #10

Closed ouotuo closed 11 years ago

ouotuo commented 11 years ago

ThreadDbSessionProvider is jodd's default SessionProvider. tomcat will work with a thread pool.when tomcat creates one thread.the thread will stay in the pool and never release the Dbsession.

there is a variable named 'wait_time' in mysql.when one connection never use for a long time,the mysql server will close it.

so,when the system never query the db for a long time.the db server closes the connection.when a query comes,one thread of tomcat in thread pool will run.it holds one Dbsession and use the Dbsession to query the db,and throw "connection is close" exception.

maybe,you say that i can use CoreConnectionPool with validateConnection=true.but in tomcat,the connection will be hold by one thread in the tomcat thread pool,and never be returned to CoreConnectionPool ,and never be checked that it is a validate connection.

thanks.

igr commented 11 years ago

We have in CoreConnectionPool the following two properties:

dbpool.validateConnection=true
dbpool.validationQuery=select 1

They are used to query database from time to time, so connection never closes by database. Would this be working for you?

See: https://github.com/oblac/uphea/blob/master/mod/app/src/app.props

(sorry for short reply, in the middle of my trip)

ouotuo commented 11 years ago

thanks.

in tomcat,DbThreadSession gets one connection from CoreConnectionPool and never be return to CoreConnectionPool.so the connection will always be held by the tomcat thread and never be validated by the CoreConnectionPool.

validation is done in the CoreConnectionPool.

igr commented 11 years ago

Ok, I think I got it what you are trying to say to me ;) Please, let me try to understand it better.

ThreadDbSessionProvider provides DbSession objects when needed, where one DbSession is shared per one thread (as thread local variable). So basically, one request will share the same DbSession object. Since threads are pooled, the same DbSession object will be used for many requests, but not in the same time, of course.

Connection is actually encapsulated by DbSession object. So, opening/closing session will fetch/return real db connection to database pool, which is CoreConnectionPool. As you noted, only returned connections are validated.

So if you open and close DbSession, everything should be ok - on DbSession#close() the connection is returned to the connection pool.

But I believe that the problem you are talking about is if you don't use DbSession directly! For example, if you create a DbQuery without specifying the session, Jodd will ask current session provider for session. However, calling DbQuery#close() never calls DbSession#close() - it only removes current query from the session. In this case the DbSession will never release the connection back to the pool.

I believe this is what is going on - good catch! Personally, I am using Jodd JTX with annotations all the time, and I didn't notice this before.

The solution would be to close DbSession explicitly on closing DbQuery when there are no more remaining associated queries.

igr commented 11 years ago

Please note that described behavior is kind of expected, and as it says in javadoc:

If thread db session is created by provider, once when not needed, session has to be closed explicitly. Session may be get by ThreadDbSessionHolder.

There are some static methods that can be used for this. Anyhow, I will change this behavior a little, so when you close DbQuery, to close the DbSession as well, also having in mind to remove session from the thread local variable, since threads are pooled; so next time new session can be created. But then this session is not shared among the threads, only if query is not closed.

igr commented 11 years ago

Im still trying to figure if this is a bug or feature :)

If we add session closing on DbQuery.close() then two DbQueries executed one after the other will be executed in two different sessions - two transactions if DbJtxSessionProvider is used, and that is definitely not something what we want.

If we require session to be closed manually, then too much magic is involved as you will have something like (sudo-code):

DbQuery dbq1 = ....
dbq1.close();
DbQuery dbq2 = ....
dbq2.close();
DbManager.getInstance().getSessionProvider().closeDbSession();

Now, we can probably fix ThreadDbSessionProvider to works better and to close session when no DbQuery is using it, so in your case not to make problems (and not to use session provider to close sessions). And I don't like that long close-db-session line out there; but there should be some mark where session ends (when not using annotations)

igr commented 11 years ago

So, the key point here is how to mark the session end in the best way.

Alternatively, we might remove SessionProvider and replace it with some kind of SessionHolder, where user creates and ends sessions manually, but does not have to use it during DbQuery creation.

ouotuo commented 11 years ago

i have fixed the problem.the process is complex.

The point of the problem is that the ThreadDbSessionProvider does not provide a valid connection,not that the user does not close the session.The connection provided by ThreadDbSessionProvider may be closed by the db server.

we can add one variable named "lastGetConnection" marked the last time .that the connection in DbThreadSession is used.when asking DbThreadSession for a connection,DbThreadSession update the variable .so we add following codes in the method "getConnection" in the DbThreadSession.

public Connection getConnection() {
        lastGetConnection = System.currentTimeMillis();
        return this.connection;
}

we also add one variable named "validationTimeout" in the ThreadDbSessionProvider .It is the same value as the "validationTimeout" variable in the CoreConnectionPool. we have to check timeout in the ThreadDbSessionProvider ,and modify the code following:

ThreadDbSessionProvider

public class ThreadDbSessionProvider implements DbSessionProvider {
    private static final Logger log = LoggerFactory
            .getLogger(ThreadDbSessionProvider.class);
    protected final boolean createIfMissing;
    private long validationTimeout = 3600000;// default one hour

    public ThreadDbSessionProvider() {
        this(true);
    }

    public MyThreadDbSessionProvider(boolean createIfMissing) {
        this.createIfMissing = createIfMissing;
    }
    public DbSession getDbSession() {
        if (log.isDebugEnabled()) {
            log.debug("Requesting thread session");
        }
        DbThreadSession session = ThreadDbSessionHolder.get();
        boolean validate=false;
        if(session!=null && session.getLastGetConnection()+validationTimeout<System.currentTimeMillis())  {
            log.debug("validate the connection");
            validate=true;
        }

        if (validate || session == null) {
            if (this.createIfMissing) {
                return new DbThreadSession(validate);
            }
            throw new DbSqlException("No session associated to current thread.");
        }
        return session;
    }
        .......
}

DbThreadSession

public class DbThreadSession extends DbSession {
    private static final Logger log = LoggerFactory
            .getLogger(ThreadSession.class);

    private long lastGetConnection = System.currentTimeMillis();

    public DbThreadSession(ConnectionProvider connectionProvider,
            boolean validate) {
        super(connectionProvider);
        DbThreadSession session = ThreadDbSessionHolder.get();
        if (session != null) {
            if (validate)
                session.closeSessionWithValidate();
            else
                session.closeSession();
        }

        ThreadDbSessionHolder.set(this);
    }

    public DbThreadSession(boolean validate) {
        this(null, validate);
    }
        ...............
        public void closeSession() {
        closeSession(false);
    }

        public void closeSessionWithValidate() {
        closeSession(true);
    }

    @SuppressWarnings({ "unchecked", "rawtypes" })
    public void closeSession(boolean validate) {
        ThreadDbSessionHolder.remove();

        log.debug("Closing db session");
        List allsexs = null;
        for (DbQueryBase query : queries) {
            List sexs = query.closeQuery();
            if (sexs != null) {
                if (allsexs == null) {
                    allsexs = new ArrayList();
                }
                allsexs.addAll(sexs);
            }
        }
        if (this.connection != null) {
            if (this.txActive == true) {
                throw new DbSqlException(
                        "Transaction was not closed before closing the session.");
            }

            if (validate) {
                                //close with validate
                 connectionProvider.closeConnection(
                        connection, true);
            } else {
                this.connectionProvider.closeConnection(connection);
            }
            this.connection = null;
        }
        this.queries = null;
        if (allsexs != null)
            throw new DbSqlException("Unable to close DbSession.", allsexs);
    }

ConnectionProvider

public abstract interface ConnectionProvider
{
  public abstract void init();

  public abstract Connection getConnection();

  public abstract void closeConnection(Connection paramConnection);

  public abstract void closeConnection(Connection paramConnection,boolean validate);

  public abstract void close();
}

CoreConnectionPool

public class CoreConnectionPool
  implements Runnable, ConnectionProvider
{
        ....
        public synchronized void closeConnection(Connection connection,boolean validate) {
        ConnectionData connectionData = new ConnectionData(connection);
        if(validate){
            connectionData.lastUsed=0;
        }

        this.busyConnections.remove(connectionData);
        this.availableConnections.add(connectionData);
        notifyAll();
    }
        ......
igr commented 11 years ago

I see your point here :) However, things are little bit different now, since 3.4.1

DbSessionProvider implementations should not create new database sessions! Instead, session should be created (and later closed) outside, as provider don't know how long the session should last.

This being said, new ThreadDbSessionProvider now just returns whatever session is associated with the current thread - it does not create session by itself anymore. The easiest way to use it is via DbThreadSession. The workflow should be like this:

DbSession dbsession = new DbThreadSession(cp);
// session is created and associated to current thread
....
DbQuery q = new DbQuery("select....");
// session is not provided explicitly, but via *ThreadDbSessionProvider*
....
DbQuery q2 = ....
// same session, another query
....
dbsession.close();
// session is close, but also removed from the thread!

Now the connection is returned to the connection pool, and the *CoreConnectionPool will validate it using existing logic and settings for that.

Note that now session is associated with the current thread only while it is active, i.e. in use. From the moment when (thread) session is closed, thread local variable is empty and can be pooled - and used again.

I believe this is a simpler solution, and it unifies DbSessionProvider implementation and behavior. See: https://github.com/oblac/jodd/blob/master/jodd-db/src/test/java/jodd/db/DbSessionProviderTest.java