On Thu, Dec 10, 2020 at 7:14 AM Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> wrote: > > + /* We only look for active and open remote > > connections. */ > > + if (entry->invalidated || !entry->conn) > > + continue; > > > > We should return even invalidated entry because it has still cached > > connection? > > > > I checked this point earlier, for invalidated connections, the tuple > returned from the cache is also invalid and the following error will > be thrown. So, we can not get the server name for that user mapping. > Cache entries too would have been invalidated after the connection is > marked as invalid in pgfdw_inval_callback(). > > umaptup = SearchSysCache1(USERMAPPINGOID, ObjectIdGetDatum(entry->key)); > if (!HeapTupleIsValid(umaptup)) > elog(ERROR, "cache lookup failed for user mapping with OID %u", > entry->key); >
I further checked on returning invalidated connections in the output of the function. Actually, the reason I'm seeing a null tuple from sys cache (and hence the error "cache lookup failed for user mapping with OID xxxx") for an invalidated connection is that the user mapping (with OID entry->key that exists in the cache) is getting dropped, so the sys cache returns null tuple. The use case is as follows: 1) Create a server, role, and user mapping of the role with the server 2) Run a foreign table query, so that the connection related to the server gets cached 3) Issue DROP OWNED BY for the role created, since the user mapping is dependent on that role, it gets dropped from the catalogue table and an invalidation message will be pending to clear the sys cache associated with that user mapping. 4) Now, if I do select * from postgres_fdw_get_connections() or for that matter any query, at the beginning the txn AtStart_Cache()-->AcceptInvalidationMessages()-->pgfdw_inval_callback() gets called and marks the cached entry as invalidated. Remember the reason for this invalidation message is that the user mapping with the OID entry->key is dropped from 3). Later in postgres_fdw_get_connections(), when we search the sys cache with entry->key for that invalidated connection, since the user mapping is dropped from the system, null tuple is returned. If we were to show invalidated connections in the output of postgres_fdw_get_connections(), we can ignore the entry and continue further if the user mapping sys cache search returns null tuple: umaptup = SearchSysCache1(USERMAPPINGOID, ObjectIdGetDatum(entry->key)); if (!HeapTupleIsValid(umaptup)) continue; Thoughts? > > Also this makes me wonder if we should return both the server name and > > boolean flag indicating whether it's invalidated or not. If so, users can > > easily find the invalidated connection entry and disconnect it because > > there is no need to keep invalidated connection. > > > > Currently we are returning a list of foreing server names with whom > there exist active connections. If we somehow address the above > mentioned problem for invalid connections and choose to show them as > well, then how should our output look like? Is it something like we > prepare a list of pairs (servername, validflag)? If agreed on above point, we can output something like: (myserver1, valid), (myserver2, valid), (myserver3, invalid), (myserver4, valid) .... Thoughts? With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com
