You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by "pivotal-jbarrett (GitHub)" <gi...@apache.org> on 2019/11/22 04:44:12 UTC

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #554: GEODE-7490: Support Visual Studio 2019

Let's remember that when fixing something in a method to cleanup the rest of the method and pay down some debt.
Changing the whole method to something like this:
```c++
void ThinClientStickyManager::cleanStaleStickyConnection() {
  LOGDEBUG("Cleaning sticky connections");
  std::lock_guard<decltype(m_stickyLock)> keysGuard(m_stickyLock);

  auto maxConnLimit = false;
  std::set<ServerLocation> excludeServers;
  for (auto it = m_stickyConnList.begin(); it != m_stickyConnList.end();) {
    auto conn = (*it);
    if (*conn) {
      if ((*conn)->setAndGetBeingUsed(true, false) &&
          canThisConnBeDeleted(*conn)) {
        auto err = GF_NOERR;
        if (auto temp = m_dm->getConnectionFromQueue(
                false, &err, excludeServers, maxConnLimit)) {
          auto temp1 = *conn;
          //*conn = temp; instead of setting in thread local put in queue,
          //thread
          // will come and pick it from there
          *conn = nullptr;
          m_dm->put(temp, false);
          temp1->close();
          _GEODE_SAFE_DELETE(temp1);
          m_dm->removeEPConnections(1, false);
          LOGDEBUG("Replaced a sticky connection");
        } else {
          (*conn)->setAndGetBeingUsed(false, false);
        }
      }
      ++it;
    } else {
      it = m_stickyConnList.erase(it);
    }
  }
}
```
Eliminates an extra pass through the `std::set` to find `nullptr`. It also eliminates the `::isNULL` method entirely.

[ Full content available at: https://github.com/apache/geode-native/pull/554 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org