Skip to content

Commit

Permalink
Add connection pool for OGR provider (Funded by Sourcepole QGIS Enter…
Browse files Browse the repository at this point in the history
…prise)
  • Loading branch information
manisandro committed May 23, 2015
1 parent 2c5c866 commit b4f4663
Show file tree
Hide file tree
Showing 9 changed files with 191 additions and 13 deletions.
27 changes: 25 additions & 2 deletions src/core/qgsconnectionpool.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
* - void qgsConnectionPool_ConnectionCreate(QString name, T& c) ... create a new connection
* - void qgsConnectionPool_ConnectionDestroy(T c) ... destroy the connection
* - QString qgsConnectionPool_ConnectionToName(T c) ... lookup connection's name (path)
* - void qgsConnectionPool_InvalidateConnection(T c) ... flag a connection as invalid
* - bool qgsConnectionPool_ConnectionIsValid(T c) ... return whether a connection is valid
*
* Because of issues with templates and QObject's signals and slots, this class only provides helper functions for QObject-related
* functionality - the place which uses the template is resonsible for:
Expand Down Expand Up @@ -87,6 +89,11 @@ class QgsConnectionPoolGroup
if ( !conns.isEmpty() )
{
Item i = conns.pop();
if ( !qgsConnectionPool_ConnectionIsValid( i.c ) )
{
qgsConnectionPool_ConnectionDestroy( i.c );
qgsConnectionPool_ConnectionCreate( connInfo, i.c );
}

// no need to run if nothing can expire
if ( conns.isEmpty() )
Expand All @@ -95,6 +102,8 @@ class QgsConnectionPoolGroup
QMetaObject::invokeMethod( expirationTimer->parent(), "stopExpirationTimer" );
}

acquiredConns.append( i.c );

return i.c;
}
}
Expand All @@ -108,11 +117,14 @@ class QgsConnectionPoolGroup
return 0;
}

acquiredConns.append( c );
return c;
}

void release( T conn )
{
acquiredConns.removeAll( conn );

connMutex.lock();
Item i;
i.c = conn;
Expand All @@ -130,6 +142,18 @@ class QgsConnectionPoolGroup
sem.release(); // this can unlock a thread waiting in acquire()
}

void invalidateConnections()
{
connMutex.lock();
foreach ( Item i, conns )
{
qgsConnectionPool_InvalidateConnection( i.c );
}
foreach ( T c, acquiredConns )
qgsConnectionPool_InvalidateConnection( c );
connMutex.unlock();
}

protected:

void initTimer( QObject* parent )
Expand Down Expand Up @@ -174,6 +198,7 @@ class QgsConnectionPoolGroup

QString connInfo;
QStack<Item> conns;
QList<T> acquiredConns;
QMutex connMutex;
QSemaphore sem;
QTimer* expirationTimer;
Expand Down Expand Up @@ -232,8 +257,6 @@ class QgsConnectionPool

protected:
T_Groups mGroups;

private:
QMutex mMutex;
};

Expand Down
4 changes: 2 additions & 2 deletions src/providers/ogr/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@

SET (OGR_SRCS qgsogrprovider.cpp qgsogrdataitems.cpp qgsogrfeatureiterator.cpp qgsogrgeometrysimplifier.cpp)
SET (OGR_SRCS qgsogrprovider.cpp qgsogrdataitems.cpp qgsogrfeatureiterator.cpp qgsogrgeometrysimplifier.cpp qgsogrconnpool.cpp)

SET(OGR_MOC_HDRS qgsogrprovider.h qgsogrdataitems.h)
SET(OGR_MOC_HDRS qgsogrprovider.h qgsogrdataitems.h qgsogrconnpool.h)

########################################################
# Build
Expand Down
41 changes: 41 additions & 0 deletions src/providers/ogr/qgsogrconnpool.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/***************************************************************************
qgsogrconnpool.cpp
---------------------
begin : May 2015
copyright : (C) 2015 by Sandro Mani
email : smani at sourcepole dot ch
***************************************************************************
* *
* This program is free software; you can redistribute it and/or modify *
* it under the terms of the GNU General Public License as published by *
* the Free Software Foundation; either version 2 of the License, or *
* (at your option) any later version. *
* *
***************************************************************************/

#include "qgsogrconnpool.h"


QgsOgrConnPool* QgsOgrConnPool::instance()
{
static QgsOgrConnPool sInstance;
return &sInstance;
}

QgsOgrConnPool::QgsOgrConnPool() : QgsConnectionPool<QgsOgrConn*, QgsOgrConnPoolGroup>()
{
QgsDebugCall;
}

QgsOgrConnPool::~QgsOgrConnPool()
{
QgsDebugCall;
}

void QgsOgrConnPool::invalidateHandles( const QString& connInfo )
{
mMutex.lock();
if ( mGroups.contains( connInfo ) )
mGroups[connInfo]->invalidateConnections();
mMutex.unlock();
}
93 changes: 93 additions & 0 deletions src/providers/ogr/qgsogrconnpool.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/***************************************************************************
qgsogrconnpool.h
---------------------
begin : May 2015
copyright : (C) 2015 by Sandro Mani
email : smani at sourcepole dot ch
***************************************************************************
* *
* This program is free software; you can redistribute it and/or modify *
* it under the terms of the GNU General Public License as published by *
* the Free Software Foundation; either version 2 of the License, or *
* (at your option) any later version. *
* *
***************************************************************************/

#ifndef QGSOGRCONNPOOL_H
#define QGSOGRCONNPOOL_H

#include "qgsconnectionpool.h"
#include <ogr_api.h>


struct QgsOgrConn
{
QString path;
OGRDataSourceH ds;
bool valid;
};

inline QString qgsConnectionPool_ConnectionToName( QgsOgrConn* c )
{
return c->path;
}

inline void qgsConnectionPool_ConnectionCreate( QString connInfo, QgsOgrConn*& c )
{
c = new QgsOgrConn;
c->ds = OGROpen( connInfo.toUtf8().constData(), false, NULL );
c->path = connInfo;
c->valid = true;
}

inline void qgsConnectionPool_ConnectionDestroy( QgsOgrConn* c )
{
OGR_DS_Destroy( c->ds );
delete c;
}

inline void qgsConnectionPool_InvalidateConnection( QgsOgrConn* c )
{
c->valid = false;
}

inline bool qgsConnectionPool_ConnectionIsValid( QgsOgrConn* c )
{
return c->valid;
}

class QgsOgrConnPoolGroup : public QObject, public QgsConnectionPoolGroup<QgsOgrConn*>
{
Q_OBJECT

public:
QgsOgrConnPoolGroup( QString name ) : QgsConnectionPoolGroup<QgsOgrConn*>( name ) { initTimer( this ); }

protected slots:
void handleConnectionExpired() { onConnectionExpired(); }
void startExpirationTimer() { expirationTimer->start(); }
void stopExpirationTimer() { expirationTimer->stop(); }

protected:
Q_DISABLE_COPY( QgsOgrConnPoolGroup )

};

/** Ogr connection pool - singleton */
class QgsOgrConnPool : public QgsConnectionPool<QgsOgrConn*, QgsOgrConnPoolGroup>
{
public:
static QgsOgrConnPool* instance();

void invalidateHandles( const QString& connInfo );

protected:
Q_DISABLE_COPY( QgsOgrConnPool )

private:
QgsOgrConnPool();
~QgsOgrConnPool();
};


#endif // QGSOGRCONNPOOL_H
15 changes: 7 additions & 8 deletions src/providers/ogr/qgsogrfeatureiterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,27 +35,26 @@

QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource* source, bool ownSource, const QgsFeatureRequest& request )
: QgsAbstractFeatureIteratorFromSource<QgsOgrFeatureSource>( source, ownSource, request )
, ogrDataSource( 0 )
, ogrLayer( 0 )
, mSubsetStringSet( false )
, mGeometrySimplifier( NULL )
{
mFeatureFetched = false;

ogrDataSource = OGROpen( TO8F( mSource->mFilePath ), false, NULL );
mConn = QgsOgrConnPool::instance()->acquireConnection( mSource->mFilePath );

if ( mSource->mLayerName.isNull() )
{
ogrLayer = OGR_DS_GetLayer( ogrDataSource, mSource->mLayerIndex );
ogrLayer = OGR_DS_GetLayer( mConn->ds, mSource->mLayerIndex );
}
else
{
ogrLayer = OGR_DS_GetLayerByName( ogrDataSource, TO8( mSource->mLayerName ) );
ogrLayer = OGR_DS_GetLayerByName( mConn->ds, TO8( mSource->mLayerName ) );
}

if ( !mSource->mSubsetString.isEmpty() )
{
ogrLayer = QgsOgrUtils::setSubsetString( ogrLayer, ogrDataSource, mSource->mEncoding, mSource->mSubsetString );
ogrLayer = QgsOgrUtils::setSubsetString( ogrLayer, mConn->ds, mSource->mEncoding, mSource->mSubsetString );
mSubsetStringSet = true;
}

Expand Down Expand Up @@ -215,13 +214,13 @@ bool QgsOgrFeatureIterator::close()

if ( mSubsetStringSet )
{
OGR_DS_ReleaseResultSet( ogrDataSource, ogrLayer );
OGR_DS_ReleaseResultSet( mConn->ds, ogrLayer );
}

OGR_DS_Destroy( ogrDataSource );
QgsOgrConnPool::instance()->releaseConnection( mConn );
mConn = 0;

mClosed = true;
ogrDataSource = 0;
return true;
}

Expand Down
3 changes: 2 additions & 1 deletion src/providers/ogr/qgsogrfeatureiterator.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#define QGSOGRFEATUREITERATOR_H

#include "qgsfeatureiterator.h"
#include "qgsogrconnpool.h"

#include <ogr_api.h>

Expand Down Expand Up @@ -71,7 +72,7 @@ class QgsOgrFeatureIterator : public QgsAbstractFeatureIteratorFromSource<QgsOgr

bool mFeatureFetched;

OGRDataSourceH ogrDataSource;
QgsOgrConn* mConn;
OGRLayerH ogrLayer;

bool mSubsetStringSet;
Expand Down
3 changes: 3 additions & 0 deletions src/providers/ogr/qgsogrprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,7 @@ OGRwkbGeometryType QgsOgrProvider::getOgrGeomType( OGRLayerH ogrLayer )

void QgsOgrProvider::loadFields()
{
QgsOgrConnPool::instance()->invalidateHandles( filePath() );
//the attribute fields need to be read again when the encoding changes
mAttributeFields.clear();

Expand Down Expand Up @@ -2522,6 +2523,8 @@ void QgsOgrProvider::recalculateFeatureCount()
{
OGR_L_SetSpatialFilter( ogrLayer, filter );
}

QgsOgrConnPool::instance()->invalidateHandles( filePath() );
}

OGRwkbGeometryType QgsOgrProvider::ogrWkbSingleFlatten( OGRwkbGeometryType type )
Expand Down
9 changes: 9 additions & 0 deletions src/providers/postgres/qgspostgresconnpool.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,15 @@ inline void qgsConnectionPool_ConnectionDestroy( QgsPostgresConn* c )
c->unref(); // will delete itself
}

inline void qgsConnectionPool_InvalidateConnection( QgsPostgresConn* c )
{
}

inline bool qgsConnectionPool_ConnectionIsValid( QgsPostgresConn* c )
{
return true;
}


class QgsPostgresConnPoolGroup : public QObject, public QgsConnectionPoolGroup<QgsPostgresConn*>
{
Expand Down
9 changes: 9 additions & 0 deletions src/providers/spatialite/qgsspatialiteconnpool.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,15 @@ inline void qgsConnectionPool_ConnectionDestroy( QgsSqliteHandle* c )
QgsSqliteHandle::closeDb( c ); // will delete itself
}

inline void qgsConnectionPool_InvalidateConnection( QgsSqliteHandle* c )
{
}

inline bool qgsConnectionPool_ConnectionIsValid( QgsSqliteHandle* c )
{
return true;
}


class QgsSpatiaLiteConnPoolGroup : public QObject, public QgsConnectionPoolGroup<QgsSqliteHandle*>
{
Expand Down

1 comment on commit b4f4663

@nirvn
Copy link
Contributor

@nirvn nirvn commented on b4f4663 Apr 10, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@manisandro , I believe this regression ( http://hub.qgis.org/issues/14560 ) is caused by connection pooling for OGR provider. Basically, when you load a singular dataset onto two layers of different geometry type (e.g. polygon and point), the feature fetching process gets messed up.

The above-refered issue has a test project file and straight forward steps to reproduce the issue. Hope that can be of help. As it stands, it'll be a regression when people upgrade from 2.8 LTS to 2.14 LTS.

Please sign in to comment.