Permalink
Browse files

make sure we lock the cache shards while we clean them, closing #1910.…

… Plus add regression test that pretty reliably

detects us not locking.
  • Loading branch information...
1 parent 7d0f392 commit 135db510604a83127283840a0373ab34d3e03da1 @ahupowerdns ahupowerdns committed with mind04 Dec 7, 2014
Showing with 60 additions and 6 deletions.
  1. +8 −4 pdns/packetcache.cc
  2. +52 −2 pdns/test-packetcache_cc.cc
View
@@ -337,7 +337,6 @@ int PacketCache::size()
/** readlock for figuring out which iterators to delete, upgrade to writelock when actually cleaning */
void PacketCache::cleanup()
{
-
*d_statnumentries=AtomicCounter(0);
BOOST_FOREACH(MapCombo& mc, d_maps) {
ReadLock l(&mc.d_mut);
@@ -364,7 +363,9 @@ void PacketCache::cleanup()
// cerr<<"cacheSize: "<<cacheSize<<", lookAt: "<<lookAt<<", toTrim: "<<toTrim<<endl;
time_t now=time(0);
DLOG(L<<"Starting cache clean"<<endl);
+ //unsigned int totErased=0;
BOOST_FOREACH(MapCombo& mc, d_maps) {
+ WriteLock wl(&mc.d_mut);
typedef cmap_t::nth_index<1>::type sequence_t;
sequence_t& sidx=mc.d_map.get<1>();
unsigned int erased=0, lookedAt=0;
@@ -373,18 +374,21 @@ void PacketCache::cleanup()
sidx.erase(i++);
erased++;
}
- else
+ else {
++i;
+ }
if(toTrim && erased > toTrim / d_maps.size())
break;
if(lookedAt > lookAt / d_maps.size())
break;
}
+ //totErased += erased;
}
- // cerr<<"erased: "<<erased<<endl;
-
+ // if(totErased)
+ // cerr<<"erased: "<<totErased<<endl;
+
*d_statnumentries=AtomicCounter(0);
BOOST_FOREACH(MapCombo& mc, d_maps) {
ReadLock l(&mc.d_mut);
@@ -101,6 +101,7 @@ catch(PDNSException& e) {
}
+
BOOST_AUTO_TEST_CASE(test_PacketCacheThreaded) {
try {
PacketCache PC;
@@ -120,9 +121,10 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheThreaded) {
for(int i=0; i < 4 ; ++i)
pthread_join(tid[i], &res);
- BOOST_CHECK_EQUAL(S.read("deferred-cache-inserts"), g_missing);
- BOOST_CHECK_EQUAL(S.read("deferred-cache-lookup"), 0);
+ BOOST_CHECK(S.read("deferred-cache-inserts") + S.read("deferred-cache-lookup") >= g_missing);
+ // BOOST_CHECK_EQUAL(S.read("deferred-cache-lookup"), 0); // cache cleaning invalidates this
+
}
catch(PDNSException& e) {
cerr<<"Had error: "<<e.reason<<endl;
@@ -131,5 +133,53 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheThreaded) {
}
+bool g_stopCleaning;
+static void *cacheCleaner(void*)
+try
+{
+ while(!g_stopCleaning) {
+ g_PC->cleanup();
+ }
+
+ return 0;
+}
+catch(PDNSException& e) {
+ cerr<<"Had error in threadReader: "<<e.reason<<endl;
+ throw;
+}
+
+BOOST_AUTO_TEST_CASE(test_PacketCacheClean) {
+ try {
+ PacketCache PC;
+
+ for(unsigned int counter = 0; counter < 1000000; ++counter) {
+ PC.insert("hello "+boost::lexical_cast<string>(counter), QType(QType::A), PacketCache::QUERYCACHE, "something", 1, 1);
+ }
+
+ sleep(1);
+
+ g_PC=&PC;
+ pthread_t tid[4];
+
+ ::arg().set("max-cache-entries")="10000";
+
+ pthread_create(&tid[0], 0, threadReader, (void*)(0*1000000UL));
+ pthread_create(&tid[1], 0, threadReader, (void*)(1*1000000UL));
+ pthread_create(&tid[2], 0, threadReader, (void*)(2*1000000UL));
+ // pthread_create(&tid[2], 0, threadMangler, (void*)(0*1000000UL));
+ pthread_create(&tid[3], 0, cacheCleaner, 0);
+
+ void *res;
+ for(int i=0; i < 3 ; ++i)
+ pthread_join(tid[i], &res);
+ g_stopCleaning=true;
+ pthread_join(tid[3], &res);
+ }
+ catch(PDNSException& e) {
+ cerr<<"Had error in threadReader: "<<e.reason<<endl;
+ throw;
+ }
+}
+
BOOST_AUTO_TEST_SUITE_END()

0 comments on commit 135db51

Please sign in to comment.