Link to home
Start Free TrialLog in
Avatar of gbcbr
gbcbrFlag for Cyprus

asked on

Not closed cursors

I have problem with permanent increasing open inactive cursors.
I suppose they are generated by not properly closed sql queries.
As I see in >> java.sql
Interface ResultSet
A ResultSet object is automatically closed when the Statement object that generated it is closed, re-executed, or used to retrieve the next result from a sequence of multiple results. <<
ResultSet has to close automatically , when next ResultSet open, apart of closing by rs.close();.
PreparedStatement closed by pstat.close();
Please advice from where may be generated unclosed cursors?  
public class ConnectDB {

    Statement statement;
    ResultSet rs;
    PreparedStatement ps;
    double[] outY = new double[10];
    Timestamp[] timest = new Timestamp[10];
    private ConnectionPool pool = null;

    public ConnectDB() throws SQLException {
        pool = new ConnectionPool("jdbc:oracle:thin:@10.1.1.7:1521:dbfx", "xx", "xx");
    }

    public ResultSet select(String sql) throws ClassNotFoundException, InstantiationException, IllegalAccessException, SQLException {
        Connection conn = pool.checkout();

        System.out.println(" Pool =>  " + pool + " : " + new java.util.Date());
        System.out.println(" Conn =>  " + conn + " : " + new java.util.Date());
        ps = conn.prepareStatement(sql);
        rs = ps.executeQuery();
        System.out.println(" ResultSet =>  " + rs + " : " + new java.util.Date());
        pool.checkin(conn);
        return rs;


    }

    public void insert(double[] outY, Timestamp[] timest) throws SQLException {

        Connection conn = pool.checkout();
        PreparedStatement pstat = conn.prepareStatement("INSERT INTO RBF_DATA (OUTY0, OUTY1, OUTY2, OUTY3, OUTY4, OUTY5, OUTY6, OUTY7, OUTY8, OUTY9, TIMEST) VALUES (?,?,?,?,?,?,?,?,?,?,?)");
        for (int i = 0; i <= 9; i++) {
            pstat.setFloat(i + 1, (float) outY[i]);
        }
        pstat.setTimestamp(11, timest[0]);
        ResultSet rset = pstat.executeQuery();
        System.out.println(" PreparedStatement =>  " + pstat + " : " + new java.util.Date());
        pstat.close();
        pool.checkin(conn);

    }
}

Open in new window

public class SelectData extends TimerTask {

    private ConnectDB conn;
    AlgoBot aBot;
    private ConnectionPool pool = null;

    public SelectData() throws SQLException, ClassNotFoundException, IllegalAccessException, InstantiationException, FieldNotFound, NotDefinedException, Exception {

        conn = new ConnectDB();
        aBot = new AlgoBot();
    }
    String[] symbol = new String[10];
    double[] openBid = new double[10];
    double[] openAsk = new double[10];
    Timestamp[] timest = new Timestamp[10];

    public void run() {

        ResultSet rs = null;

        try {
            rs =
                    conn.select("SELECT * FROM EURUSD where timest = ( select max( timest ) from EURUSD ) and rownum = 1 union all "
                    + "SELECT * FROM EURCHF where timest = ( select max( timest ) from EURCHF ) and rownum = 1 union all "
                    + "SELECT * FROM EURGBP where timest = ( select max( timest ) from EURGBP ) and rownum = 1 union all "
                    + "SELECT * FROM USDCHF where timest = ( select max( timest ) from USDCHF ) and rownum = 1 union all "
                    + "SELECT * FROM GBPUSD where timest = ( select max( timest ) from GBPUSD ) and rownum = 1 union all "
                    + "SELECT * FROM GBPCHF where timest = ( select max( timest ) from GBPCHF ) and rownum = 1 union all "
                    + "SELECT * FROM EURJPY where timest = ( select max( timest ) from EURJPY ) and rownum = 1 union all "
                    + "SELECT * FROM USDJPY where timest = ( select max( timest ) from USDJPY ) and rownum = 1 union all "
                    + "SELECT * FROM GBPJPY where timest = ( select max( timest ) from GBPJPY ) and rownum = 1 union all "
                    + "SELECT * FROM CHFJPY where timest = ( select max( timest ) from CHFJPY ) and rownum = 1");

            while (rs.next()) {

                String s = rs.getString("symbol");
                Timestamp ts = rs.getTimestamp("TIMEST");
                double bpx = rs.getDouble("BIDPX");
                double apx = rs.getDouble("ASKPX");

                if (s.equals("EUR/USD")) {

                    timest[0] = ts;
                    openBid[0] = bpx;
                    openAsk[0] = apx;

                } else if (s.equals("EUR/CHF")) {

                    timest[1] = ts;
                    openBid[1] = bpx;
                    openAsk[1] = apx;

                } else if (s.equals("EUR/GBP")) {

                    timest[2] = ts;
                    openBid[2] = bpx;
                    openAsk[2] = apx;

                } else if (s.equals("USD/CHF")) {

                    timest[3] = ts;
                    openBid[3] = bpx;
                    openAsk[3] = apx;

                } else if (s.equals("GBP/USD")) {

                    timest[4] = ts;
                    openBid[4] = bpx;
                    openAsk[4] = apx;

                } else if (s.equals("GBP/CHF")) {

                    timest[5] = ts;
                    openBid[5] = bpx;
                    openAsk[5] = apx;

                } else if (s.equals("EUR/JPY")) {

                    timest[6] = ts;
                    openBid[6] = bpx;
                    openAsk[6] = apx;

                } else if (s.equals("USD/JPY")) {

                    timest[7] = ts;
                    openBid[7] = bpx;
                    openAsk[7] = apx;

                } else if (s.equals("GBP/JPY")) {

                    timest[8] = ts;
                    openBid[8] = bpx;
                    openAsk[8] = apx;

                } else if (s.equals("CHF/JPY")) {

                    timest[9] = ts;
                    openBid[9] = bpx;
                    openAsk[9] = apx;

                    equals(rs);

                    aBot.algoBot(openBid, openAsk, timest);

                } else {
                }
            }


            System.out.println(" ResultSet 1 =>  " + rs + " : " + new java.util.Date());

            rs.close();
            System.out.println(" ResultSet 2 =>  " + rs + " : " + new java.util.Date());

        } catch (Exception ex) {
            Logger.getLogger(SelectData.class.getName()).log(Level.SEVERE,
                    null, ex);
        } finally {
            try {
                if (rs != null) {
                    rs.close();
                    System.out.println(" ResultSet 3 =>  " + rs + " : " + new java.util.Date());
                }
            } catch (SQLException sqle) {
                Logger.getLogger(SelectData.class.getName()).log(Level.SEVERE,
                        "Error while closing resultset", sqle);
            }
        }

Open in new window

Avatar of slightwv (䄆 Netminder)
slightwv (䄆 Netminder)

Is this really not a duplicate of a previous question you closed out?

https://www.experts-exchange.com/questions/26819217/DB-connection-problem.html

Avatar of gbcbr

ASKER

No, previous question was about <<ORA-12519, TNS:no appropriate service handler found>>
stacking system
Now system works, but with crazy 5 minutes timer error.
Exactly 5 minutes, I can make special coffee timer on this error, and number of open cursors arise up to now 5000>
and I don't know, how much it will be tomorrow morning.
Avatar of gbcbr

ASKER

I need Java experts which can find mistake in my code
I can see at least two issues in the SelectData class:

1. Line 101 has this error (or perhaps a loose piece of code)
>>> equals(rs);

2. You are not closing the connection anywhere (the one generated via conn = new ConnectDB(); )

There's something else that I'm not sure is an actual issue but it could become one with resultsets holding large amounts of data: in ConnectDB.select(), you get a new connection from the pool, run the SQL query, return the connection to the pool and only then return the resultset.  This doesn't seem right.

Finally, I don't see where you are actually closing the connection(s), unless you are doing so at checkin time. Without seeing the ConnectionPool that you're using, it's difficult to draw any conclusions.
Avatar of gbcbr

ASKER

public class ConnectionPool implements Runnable
{
    // Number of initial connections to make.
    private int m_InitialConnectionCount = 4;
    // A list of available connections for use.
    private Vector m_AvailableConnections = new Vector();
    // A list of connections being used currently.
    private Vector m_UsedConnections = new Vector();
    // The URL string used to connect to the database
    private String m_URLString = null;
    // The username used to connect to the database
    private String m_UserName = null;
    // The password used to connect to the database
    private String m_Password = null;
    // The cleanup thread
    private Thread m_CleanupThread = null;


    //Constructor
    public ConnectionPool(String urlString, String user, String passwd) throws SQLException
    {
        // Initialize the required parameters
        m_URLString = urlString;
        m_UserName = user;
        m_Password = passwd;

        for(int cnt=0; cnt<m_InitialConnectionCount; cnt++)
        {
            // Add a new connection to the available list.
            m_AvailableConnections.addElement(getConnection());
        }

        // Create the cleanup thread
        m_CleanupThread = new Thread(this);
        m_CleanupThread.start();
    }

    private Connection getConnection() throws SQLException
    {
        return DriverManager.getConnection(m_URLString, m_UserName, m_Password);
    }

    public synchronized Connection checkout() throws SQLException
    {
        Connection newConnxn = null;

        if(m_AvailableConnections.isEmpty())
        {
            // Im out of connections. Create one more.
             newConnxn = getConnection();
            // Add this connection to the "Used" list.
             m_UsedConnections.addElement(newConnxn);
            // We dont have to do anything else since this is
            // a new connection.
        }
        else
        {
            // Connections exist !
            // Get a connection object
            newConnxn = (Connection)m_AvailableConnections.lastElement();
            // Remove it from the available list.
            m_AvailableConnections.removeElement(newConnxn);
            // Add it to the used list.
            m_UsedConnections.addElement(newConnxn);
        }

        // Either way, we should have a connection object now.
        return newConnxn;
    }


    public synchronized void checkin(Connection c)
    {
        if(c != null)
        {
            // Remove from used list.
            m_UsedConnections.removeElement(c);
            // Add to the available list
            m_AvailableConnections.addElement(c);
        }
    }

    public int availableCount()
    {
        return m_AvailableConnections.size();
    }

    public void run()
    {
        try
        {
            while(true)
            {
                synchronized(this)
                {
                    while(m_AvailableConnections.size() > m_InitialConnectionCount)
                    {
                        // Clean up extra available connections.
                        Connection c = (Connection)m_AvailableConnections.lastElement();
                        m_AvailableConnections.removeElement(c);

                        // Close the connection to the database.
                        c.close();
                    }

                    // Clean up is done
                }

                System.out.println("CLEANUP : Available Connections : " + availableCount());

                // Now sleep for 1 minute
                Thread.sleep(60000 * 1);
            }
        }
        catch(SQLException sqle)
        {
            sqle.printStackTrace();
        }
        catch(Exception e)
        {
            e.printStackTrace();
        }
    }
}

Open in new window

The PreparedStatement which you create in ConnectDB.select() remain unclosed.
My advise is to iterate the resultset in this select() method, fill a List with data from ResultSet and return this list from select(). In this way you can close the ResultSet and PreparedStatement in a finally block in select() method.
Avatar of gbcbr

ASKER

>>aciuica
can you please show this changes in my code?
I already explained this to you in a previous question
In your ConnectDB class you cannot use the ResultSet after checking in the connection. It is not safe.

And the problem with increasing cursors is most likely with the connection pool you are using.
Closing a resultset closes the statement, but it does not close the connection. The exception is caused by too many open connections, not open statements.
Avatar of gbcbr

ASKER

OK, I'm specially was waiting for your comment, because this code is your baby.
From one hand, I have to reduce number of connections to DB by creating pool, from other hand, this pool create more problems with unclosed cursors.
I have to choose:
1. left every second connect-disconnect option for select and insert methods,
2. or try to solve pool connection for both of them.
You know, I'm not an expert in Java, I need just best productivity with minimal delay, sources we have enough, 12 cores, 24GB RAM, and no other tasks.
Please advice which solution will be better for my case.
the connection pool should be limiting the number of open connections (and closing them as required)

just noticed your code doesn't handle exceptions, which could result in connections not being checked back into the pool
Avatar of gbcbr

ASKER

Connection pool has limitation = 4 connection.
>>Closing a resultset closes the statement, but it does not close the connection<<
It  has to be way to close connection together with rs.
I'm sure that I'm not a pioneer in this primitive questions. It has to be standard solutions for these standard questions.
My problem is only that I have very small experience today to find this solution, but i's 1000 times bigger than 3 month before.
Please advice practical solution for this question.
> Is this really not a duplicate of a previous question you closed out?

looks like it is the same problem
> It  has to be way to close connection together with rs.

you don't want to close the connection, thats the job of the connection pool

I'd start with using a decent connection pool such as dbcp or c3po
Avatar of gbcbr

ASKER

no, before it was blocked at all, now it works.
I lose 1 data per 5 minutes, but it's nothing for calculation(1 sec from 300)
you only fixed part of the problem in the previous question
Avatar of gbcbr

ASKER

yes, I agree, but part is better than nothing
ASKER CERTIFIED SOLUTION
Avatar of aciuica
aciuica
Flag of Romania image

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Avatar of gbcbr

ASKER

At least, I have now more or less working app and I can continue with my development.
Second question will be make it from average to good, and in the future - excellent.
Now I'm at the first stage.
Avatar of gbcbr

ASKER

Excellent!!! Please double points!
Which is exactly what I suggested to you many questions ago :)
Avatar of gbcbr

ASKER

>>objects
You are the best! Only one problem, whenI need your help, you are sleep, when you wake up, I'm sleep.
Now I have also some new very cleaver guys which help me.
Thank you for everybody who spend his time and try to teach me, old donkey, but I try to look young! Like all of your.
My point is to remember what you have been shown so you don't repeat previous mistakes.
Avatar of gbcbr

ASKER

I never step on the same rakes double, each new question I ask only after checking old rakes. Before we never had select and insert  joined in one class. This was first lesson about joining two opposite queries in one class, it takes two days of my life, but, God bless me, it takes only two days.