Skip to content

Commit 18112c0

Browse files
m-kuhnm-kuhn
committedAug 8, 2018
Rename freeConnectionsRequirement to requestMayBeNested
·
final-3_2_3final-3_2_2
1 parent 6a9c5fa commit 18112c0

File tree

5 files changed

+208
-219
lines changed

5 files changed

+208
-219
lines changed
 

‎python/core/auto_generated/qgsfeaturerequest.sip.in

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -659,36 +659,30 @@ at this moment. A negative value (which is set by default) will wait forever.
659659
.. versionadded:: 3.0
660660
%End
661661

662-
int freeConnectionsRequirement() const;
662+
bool requestMayBeNested() const;
663663
%Docstring
664-
The amount of free connections required to start this request.
665-
The system will block the request until the specified amount of connections
666-
is available for usage.
664+
In case this request may be run nested within another already running
665+
iteration on the same connection, set this to true.
667666

668-
By default this amount is 3. This makes sure, that we have 2 spare connections
669-
that might be used by "nested" requests which are executed while iterating
670-
over the results of this request.
667+
If this flag is true, this request will be able to make use of "spare"
668+
connections to avoid deadlocks.
671669

672-
This number should be changed to one, when we know that no nested requests happen
673-
and that this request might happen in a nested way. This is for example given for
674-
expression functions that do internal requests.
670+
For example, this should be set on requests that are issued from an
671+
expression function.
675672

676673
.. versionadded:: 3.4
677674
%End
678675

679-
void setFreeConnectionsRequirement( int freeConnectionsRequirement );
676+
void setRequestMayBeNested( bool requestMayBeNested );
680677
%Docstring
681-
The amount of free connections required to start this request.
682-
The system will block the request until the specified amount of connections
683-
is available for usage.
678+
In case this request may be run nested within another already running
679+
iteration on the same connection, set this to true.
684680

685-
By default this amount is 3. This makes sure, that we have 2 spare connections
686-
that might be used by "nested" requests which are executed while iterating
687-
over the results of this request.
681+
If this flag is true, this request will be able to make use of "spare"
682+
connections to avoid deadlocks.
688683

689-
This number should be changed to one, when we know that no nested requests happen
690-
and that this request might happen in a nested way. This is for example given for
691-
expression functions that do internal requests.
684+
For example, this should be set on requests that are issued from an
685+
expression function.
692686

693687
.. versionadded:: 3.4
694688
%End

‎src/core/qgsconnectionpool.h

Lines changed: 174 additions & 173 deletions
Original file line numberDiff line numberDiff line change
@@ -93,12 +93,13 @@ class QgsConnectionPoolGroup
9393
*
9494
* \returns initialized connection or nullptr if unsuccessful
9595
*/
96-
T acquire( int timeout, int freeConnectionRequirement )
96+
T acquire( int timeout, bool requestMayBeNested )
9797
{
98+
const int requiredFreeConnectionCount = requestMayBeNested ? 1 : 3;
9899
// we are going to acquire a resource - if no resource is available, we will block here
99100
if ( timeout >= 0 )
100101
{
101-
if ( !sem.tryAcquire( freeConnectionRequirement, timeout ) )
102+
if ( !sem.tryAcquire( requiredFreeConnectionCount, timeout ) )
102103
return nullptr;
103104
}
104105
else
@@ -107,9 +108,9 @@ class QgsConnectionPoolGroup
107108
// tryAcquire is broken on Qt > 5.8 with negative timeouts - see
108109
// https://bugreports.qt.io/browse/QTBUG-64413
109110
// https://lists.osgeo.org/pipermail/qgis-developer/2017-November/050456.html
110-
sem.acquire( freeConnectionRequirement );
111+
sem.acquire( requiredFreeConnectionCount );
111112
}
112-
sem.release( freeConnectionRequirement - 1 );
113+
sem.release( requiredFreeConnectionCount - 1 );
113114

114115
// quick (preferred) way - use cached connection
115116
{
@@ -122,218 +123,218 @@ class QgsConnectionPoolGroup
122123
{
123124
qgsConnectionPool_ConnectionDestroy( i.c );
124125
qgsConnectionPool_ConnectionCreate( connInfo, i.c );
125-
}
126126

127-
// no need to run if nothing can expire
128-
if ( conns.isEmpty() )
129-
{
130-
// will call the slot directly or queue the call (if the object lives in a different thread)
131-
QMetaObject::invokeMethod( expirationTimer->parent(), "stopExpirationTimer" );
132-
}
133127

134-
acquiredConns.append( i.c );
128+
// no need to run if nothing can expire
129+
if ( conns.isEmpty() )
130+
{
131+
// will call the slot directly or queue the call (if the object lives in a different thread)
132+
QMetaObject::invokeMethod( expirationTimer->parent(), "stopExpirationTimer" );
133+
}
135134

136-
return i.c;
137-
}
138-
}
135+
acquiredConns.append( i.c );
139136

140-
T c;
141-
qgsConnectionPool_ConnectionCreate( connInfo, c );
142-
if ( !c )
143-
{
144-
// we didn't get connection for some reason, so release the lock
145-
sem.release();
146-
return nullptr;
147-
}
137+
return i.c;
138+
}
139+
}
148140

149-
connMutex.lock();
150-
acquiredConns.append( c );
151-
connMutex.unlock();
152-
return c;
153-
}
141+
T c;
142+
qgsConnectionPool_ConnectionCreate( connInfo, c );
143+
if ( !c )
144+
{
145+
// we didn't get connection for some reason, so release the lock
146+
sem.release();
147+
return nullptr;
148+
}
154149

155-
void release( T conn )
156-
{
157-
connMutex.lock();
158-
acquiredConns.removeAll( conn );
159-
if ( !qgsConnectionPool_ConnectionIsValid( conn ) )
160-
{
161-
qgsConnectionPool_ConnectionDestroy( conn );
150+
connMutex.lock();
151+
acquiredConns.append( c );
152+
connMutex.unlock();
153+
return c;
162154
}
163-
else
164-
{
165-
Item i;
166-
i.c = conn;
167-
i.lastUsedTime = QTime::currentTime();
168-
conns.push( i );
169155

170-
if ( !expirationTimer->isActive() )
156+
void release( T conn )
157+
{
158+
connMutex.lock();
159+
acquiredConns.removeAll( conn );
160+
if ( !qgsConnectionPool_ConnectionIsValid( conn ) )
171161
{
172-
// will call the slot directly or queue the call (if the object lives in a different thread)
173-
QMetaObject::invokeMethod( expirationTimer->parent(), "startExpirationTimer" );
162+
qgsConnectionPool_ConnectionDestroy( conn );
163+
}
164+
else
165+
{
166+
Item i;
167+
i.c = conn;
168+
i.lastUsedTime = QTime::currentTime();
169+
conns.push( i );
170+
171+
if ( !expirationTimer->isActive() )
172+
{
173+
// will call the slot directly or queue the call (if the object lives in a different thread)
174+
QMetaObject::invokeMethod( expirationTimer->parent(), "startExpirationTimer" );
175+
}
174176
}
175-
}
176177

177-
connMutex.unlock();
178+
connMutex.unlock();
178179

179-
sem.release(); // this can unlock a thread waiting in acquire()
180-
}
180+
sem.release(); // this can unlock a thread waiting in acquire()
181+
}
181182

182-
void invalidateConnections()
183-
{
184-
connMutex.lock();
185-
for ( const Item &i : qgis::as_const( conns ) )
183+
void invalidateConnections()
186184
{
187-
qgsConnectionPool_ConnectionDestroy( i.c );
185+
connMutex.lock();
186+
for ( const Item &i : qgis::as_const( conns ) )
187+
{
188+
qgsConnectionPool_ConnectionDestroy( i.c );
189+
}
190+
conns.clear();
191+
for ( T c : qgis::as_const( acquiredConns ) )
192+
qgsConnectionPool_InvalidateConnection( c );
193+
connMutex.unlock();
188194
}
189-
conns.clear();
190-
for ( T c : qgis::as_const( acquiredConns ) )
191-
qgsConnectionPool_InvalidateConnection( c );
192-
connMutex.unlock();
193-
}
194-
195-
protected:
196195

197-
void initTimer( QObject *parent )
198-
{
199-
expirationTimer = new QTimer( parent );
200-
expirationTimer->setInterval( CONN_POOL_EXPIRATION_TIME * 1000 );
201-
QObject::connect( expirationTimer, SIGNAL( timeout() ), parent, SLOT( handleConnectionExpired() ) );
196+
protected:
202197

203-
// just to make sure the object belongs to main thread and thus will get events
204-
if ( qApp )
205-
parent->moveToThread( qApp->thread() );
206-
}
207-
208-
void onConnectionExpired()
209-
{
210-
connMutex.lock();
211-
212-
QTime now = QTime::currentTime();
213-
214-
// what connections have expired?
215-
QList<int> toDelete;
216-
for ( int i = 0; i < conns.count(); ++i )
198+
void initTimer( QObject * parent )
217199
{
218-
if ( conns.at( i ).lastUsedTime.secsTo( now ) >= CONN_POOL_EXPIRATION_TIME )
219-
toDelete.append( i );
200+
expirationTimer = new QTimer( parent );
201+
expirationTimer->setInterval( CONN_POOL_EXPIRATION_TIME * 1000 );
202+
QObject::connect( expirationTimer, SIGNAL( timeout() ), parent, SLOT( handleConnectionExpired() ) );
203+
204+
// just to make sure the object belongs to main thread and thus will get events
205+
if ( qApp )
206+
parent->moveToThread( qApp->thread() );
220207
}
221208

222-
// delete expired connections
223-
for ( int j = toDelete.count() - 1; j >= 0; --j )
209+
void onConnectionExpired()
224210
{
225-
int index = toDelete[j];
226-
qgsConnectionPool_ConnectionDestroy( conns[index].c );
227-
conns.remove( index );
228-
}
211+
connMutex.lock();
229212

230-
if ( conns.isEmpty() )
231-
expirationTimer->stop();
213+
QTime now = QTime::currentTime();
232214

233-
connMutex.unlock();
234-
}
215+
// what connections have expired?
216+
QList<int> toDelete;
217+
for ( int i = 0; i < conns.count(); ++i )
218+
{
219+
if ( conns.at( i ).lastUsedTime.secsTo( now ) >= CONN_POOL_EXPIRATION_TIME )
220+
toDelete.append( i );
221+
}
235222

236-
protected:
223+
// delete expired connections
224+
for ( int j = toDelete.count() - 1; j >= 0; --j )
225+
{
226+
int index = toDelete[j];
227+
qgsConnectionPool_ConnectionDestroy( conns[index].c );
228+
conns.remove( index );
229+
}
237230

238-
QString connInfo;
239-
QStack<Item> conns;
240-
QList<T> acquiredConns;
241-
QMutex connMutex;
242-
QSemaphore sem;
243-
QTimer *expirationTimer = nullptr;
231+
if ( conns.isEmpty() )
232+
expirationTimer->stop();
244233

245-
};
234+
connMutex.unlock();
235+
}
246236

237+
protected:
247238

248-
/**
249-
* \ingroup core
250-
* Template class responsible for keeping a pool of open connections.
251-
* This is desired to avoid the overhead of creation of new connection every time.
252-
*
253-
* The methods are thread safe.
254-
*
255-
* The connection pool has a limit on maximum number of concurrent connections
256-
* (per server), once the limit is reached, the acquireConnection() function
257-
* will block. All connections that have been acquired must be then released
258-
* with releaseConnection() function.
259-
*
260-
* When the connections are not used for some time, they will get closed automatically
261-
* to save resources.
262-
* \note not available in Python bindings
263-
*/
264-
template <typename T, typename T_Group>
265-
class QgsConnectionPool
266-
{
267-
public:
239+
QString connInfo;
240+
QStack<Item> conns;
241+
QList<T> acquiredConns;
242+
QMutex connMutex;
243+
QSemaphore sem;
244+
QTimer *expirationTimer = nullptr;
268245

269-
typedef QMap<QString, T_Group *> T_Groups;
246+
};
270247

271-
virtual ~QgsConnectionPool()
272-
{
273-
mMutex.lock();
274-
for ( T_Group *group : qgis::as_const( mGroups ) )
275-
{
276-
delete group;
277-
}
278-
mGroups.clear();
279-
mMutex.unlock();
280-
}
281248

282249
/**
283-
* Try to acquire a connection for a maximum of \a timeout milliseconds.
284-
* If \a timeout is a negative value the calling thread will be blocked
285-
* until a connection becomes available. This is the default behavior.
250+
* \ingroup core
251+
* Template class responsible for keeping a pool of open connections.
252+
* This is desired to avoid the overhead of creation of new connection every time.
286253
*
254+
* The methods are thread safe.
287255
*
256+
* The connection pool has a limit on maximum number of concurrent connections
257+
* (per server), once the limit is reached, the acquireConnection() function
258+
* will block. All connections that have been acquired must be then released
259+
* with releaseConnection() function.
288260
*
289-
* \returns initialized connection or nullptr if unsuccessful
261+
* When the connections are not used for some time, they will get closed automatically
262+
* to save resources.
263+
* \note not available in Python bindings
290264
*/
291-
T acquireConnection( const QString &connInfo, int timeout = -1, int freeConnectionRequirement = 3 )
265+
template <typename T, typename T_Group>
266+
class QgsConnectionPool
292267
{
293-
mMutex.lock();
294-
typename T_Groups::iterator it = mGroups.find( connInfo );
295-
if ( it == mGroups.end() )
296-
{
297-
it = mGroups.insert( connInfo, new T_Group( connInfo ) );
298-
}
299-
T_Group *group = *it;
300-
mMutex.unlock();
268+
public:
301269

302-
return group->acquire( timeout, freeConnectionRequirement );
303-
}
270+
typedef QMap<QString, T_Group *> T_Groups;
304271

305-
//! Release an existing connection so it will get back into the pool and can be reused
306-
void releaseConnection( T conn )
307-
{
308-
mMutex.lock();
309-
typename T_Groups::iterator it = mGroups.find( qgsConnectionPool_ConnectionToName( conn ) );
310-
Q_ASSERT( it != mGroups.end() );
311-
T_Group *group = *it;
312-
mMutex.unlock();
272+
virtual ~QgsConnectionPool()
273+
{
274+
mMutex.lock();
275+
for ( T_Group *group : qgis::as_const( mGroups ) )
276+
{
277+
delete group;
278+
}
279+
mGroups.clear();
280+
mMutex.unlock();
281+
}
313282

314-
group->release( conn );
315-
}
283+
/**
284+
* Try to acquire a connection for a maximum of \a timeout milliseconds.
285+
* If \a timeout is a negative value the calling thread will be blocked
286+
* until a connection becomes available. This is the default behavior.
287+
*
288+
*
289+
*
290+
* \returns initialized connection or nullptr if unsuccessful
291+
*/
292+
T acquireConnection( const QString &connInfo, int timeout = -1, bool requestMayBeNested = false )
293+
{
294+
mMutex.lock();
295+
typename T_Groups::iterator it = mGroups.find( connInfo );
296+
if ( it == mGroups.end() )
297+
{
298+
it = mGroups.insert( connInfo, new T_Group( connInfo ) );
299+
}
300+
T_Group *group = *it;
301+
mMutex.unlock();
316302

317-
/**
318-
* Invalidates all connections to the specified resource.
319-
* The internal state of certain handles (for instance OGR) are altered
320-
* when a dataset is modified. Consquently, all open handles need to be
321-
* invalidated when such datasets are changed to ensure the handles are
322-
* refreshed. See the OGR provider for an example where this is needed.
323-
*/
324-
void invalidateConnections( const QString &connInfo )
325-
{
326-
mMutex.lock();
327-
if ( mGroups.contains( connInfo ) )
328-
mGroups[connInfo]->invalidateConnections();
329-
mMutex.unlock();
330-
}
303+
return group->acquire( timeout, requestMayBeNested );
304+
}
331305

306+
//! Release an existing connection so it will get back into the pool and can be reused
307+
void releaseConnection( T conn )
308+
{
309+
mMutex.lock();
310+
typename T_Groups::iterator it = mGroups.find( qgsConnectionPool_ConnectionToName( conn ) );
311+
Q_ASSERT( it != mGroups.end() );
312+
T_Group *group = *it;
313+
mMutex.unlock();
314+
315+
group->release( conn );
316+
}
332317

333-
protected:
334-
T_Groups mGroups;
335-
QMutex mMutex;
336-
};
318+
/**
319+
* Invalidates all connections to the specified resource.
320+
* The internal state of certain handles (for instance OGR) are altered
321+
* when a dataset is modified. Consquently, all open handles need to be
322+
* invalidated when such datasets are changed to ensure the handles are
323+
* refreshed. See the OGR provider for an example where this is needed.
324+
*/
325+
void invalidateConnections( const QString &connInfo )
326+
{
327+
mMutex.lock();
328+
if ( mGroups.contains( connInfo ) )
329+
mGroups[connInfo]->invalidateConnections();
330+
mMutex.unlock();
331+
}
332+
333+
334+
protected:
335+
T_Groups mGroups;
336+
QMutex mMutex;
337+
};
337338

338339

339340
#endif // QGSCONNECTIONPOOL_H

‎src/core/qgsfeaturerequest.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -299,14 +299,14 @@ void QgsFeatureRequest::setConnectionTimeout( int connectionTimeout )
299299
mConnectionTimeout = connectionTimeout;
300300
}
301301

302-
int QgsFeatureRequest::freeConnectionsRequirement() const
302+
bool QgsFeatureRequest::requestMayBeNested() const
303303
{
304-
return mFreeConnectionsRequirement;
304+
return mRequestMayBeNested;
305305
}
306306

307-
void QgsFeatureRequest::setFreeConnectionsRequirement( int freeConnectionsRequirement )
307+
void QgsFeatureRequest::setRequestMayBeNested( bool requestMayBeNested )
308308
{
309-
mFreeConnectionsRequirement = freeConnectionsRequirement;
309+
mRequestMayBeNested = requestMayBeNested;
310310
}
311311

312312

‎src/core/qgsfeaturerequest.h

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -637,38 +637,32 @@ class CORE_EXPORT QgsFeatureRequest
637637
void setConnectionTimeout( int connectionTimeout );
638638

639639
/**
640-
* The amount of free connections required to start this request.
641-
* The system will block the request until the specified amount of connections
642-
* is available for usage.
640+
* In case this request may be run nested within another already running
641+
* iteration on the same connection, set this to true.
643642
*
644-
* By default this amount is 3. This makes sure, that we have 2 spare connections
645-
* that might be used by "nested" requests which are executed while iterating
646-
* over the results of this request.
643+
* If this flag is true, this request will be able to make use of "spare"
644+
* connections to avoid deadlocks.
647645
*
648-
* This number should be changed to one, when we know that no nested requests happen
649-
* and that this request might happen in a nested way. This is for example given for
650-
* expression functions that do internal requests.
646+
* For example, this should be set on requests that are issued from an
647+
* expression function.
651648
*
652649
* \since QGIS 3.4
653650
*/
654-
int freeConnectionsRequirement() const;
651+
bool requestMayBeNested() const;
655652

656653
/**
657-
* The amount of free connections required to start this request.
658-
* The system will block the request until the specified amount of connections
659-
* is available for usage.
654+
* In case this request may be run nested within another already running
655+
* iteration on the same connection, set this to true.
660656
*
661-
* By default this amount is 3. This makes sure, that we have 2 spare connections
662-
* that might be used by "nested" requests which are executed while iterating
663-
* over the results of this request.
657+
* If this flag is true, this request will be able to make use of "spare"
658+
* connections to avoid deadlocks.
664659
*
665-
* This number should be changed to one, when we know that no nested requests happen
666-
* and that this request might happen in a nested way. This is for example given for
667-
* expression functions that do internal requests.
660+
* For example, this should be set on requests that are issued from an
661+
* expression function.
668662
*
669663
* \since QGIS 3.4
670664
*/
671-
void setFreeConnectionsRequirement( int freeConnectionsRequirement );
665+
void setRequestMayBeNested( bool requestMayBeNested );
672666

673667
protected:
674668
FilterType mFilter = FilterNone;
@@ -688,7 +682,7 @@ class CORE_EXPORT QgsFeatureRequest
688682
QgsCoordinateReferenceSystem mCrs;
689683
QgsCoordinateTransformContext mTransformContext;
690684
int mConnectionTimeout = -1;
691-
int mFreeConnectionsRequirement = 3;
685+
int mRequestMayBeNested = false;
692686
};
693687

694688
Q_DECLARE_OPERATORS_FOR_FLAGS( QgsFeatureRequest::Flags )

‎src/providers/postgres/qgspostgresfeatureiterator.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ QgsPostgresFeatureIterator::QgsPostgresFeatureIterator( QgsPostgresFeatureSource
3838

3939
if ( !source->mTransactionConnection )
4040
{
41-
mConn = QgsPostgresConnPool::instance()->acquireConnection( mSource->mConnInfo, request.connectionTimeout(), request.freeConnectionsRequirement() );
41+
mConn = QgsPostgresConnPool::instance()->acquireConnection( mSource->mConnInfo, request.connectionTimeout(), request.requestMayBeNested() );
4242
mIsTransactionConnection = false;
4343
}
4444
else

0 commit comments

Comments
 (0)
Please sign in to comment.