Review AMT comments

xrootd · Oct 26, 2016 · 67b1ac5 · 67b1ac5
1 parent ff6f3d4
commit 67b1ac5
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 18 deletions.
diff --git a/src/XrdFileCache/XrdFileCache.cc b/src/XrdFileCache/XrdFileCache.cc
@@ -187,7 +187,7 @@ Cache::AddWriteTask(Block* b, bool fromRead)
    if (fromRead)
       m_writeQ.queue.push_back(b);
    else
-      m_writeQ.queue.push_front(b); // AMT should this not be the opposite
+      m_writeQ.queue.push_front(b);
    m_writeQ.size++;
    m_writeQ.condVar.Signal();
    m_writeQ.condVar.UnLock();
@@ -227,7 +227,7 @@ Cache::ProcessWriteTasks()
       {
          m_writeQ.condVar.Wait();
       }
-      Block* block = m_writeQ.queue.front(); // AMT should not be back ???
+      Block* block = m_writeQ.queue.front();
       m_writeQ.queue.pop_front();
       m_writeQ.size--;
       TRACE(Dump, "Cache::ProcessWriteTasks  for %p " <<  (void*)(block) << " path " << block->m_file->lPath());

diff --git a/src/XrdFileCache/XrdFileCacheConfiguration.cc b/src/XrdFileCache/XrdFileCacheConfiguration.cc
@@ -243,7 +243,7 @@ bool Cache::Config(XrdSysLogger *logger, const char *config_filename, const char
                         "       pfc.trace %d",
                         config_filename,
                         m_configuration.m_bufferSize,
-                        m_configuration.m_prefetch_max_blocks, // AMT not sure what parsing should be
+                        m_configuration.m_prefetch_max_blocks,
                         rg,
                         m_configuration.m_diskUsageLWM,
                         m_configuration.m_diskUsageHWM,
@@ -398,4 +398,4 @@ bool Cache::ConfigParameters(std::string part, XrdOucStream& config, TmpConfigur
     assert (config.GetWord() == 0 && "Cache::ConfigParameters() lost argument");
 
     return true;
-}
+}
diff --git a/src/XrdFileCache/XrdFileCacheFile.cc b/src/XrdFileCache/XrdFileCacheFile.cc
@@ -449,7 +449,6 @@ int File::ReadBlocksFromDisk(std::list<int>& blocks,
          return rs;
       }
 
-      // AMT I think we should exit in this case too
       if (rs != size)
       {
          TRACEF(Error, "File::ReadBlocksFromDisk incomplete size = " <<  rs << " idx = " << *ii);
@@ -550,7 +549,7 @@ int File::Read(char* iUserBuff, long long iUserOff, int iUserSize)
    {
       for (BlockList_i i = blks_to_process.begin(); i != blks_to_process.end(); ++i)
          dec_ref_count(*i);
-      return -1;   // AMT ???
+      return -1;
    }
 
    ProcessBlockRequests(blks_to_request);
@@ -573,7 +572,7 @@ int File::Read(char* iUserBuff, long long iUserOff, int iUserSize)
          for (BlockList_i i = blks_to_process.begin(); i!= blks_to_process.end(); ++i )
             dec_ref_count(*i);
          delete direct_handler;
-         return -1;   // AMT ???
+         return -1;
       }
       TRACEF(Dump, "File::Read() direct read finished, size = " << direct_size);
    }
@@ -591,8 +590,7 @@ int File::Read(char* iUserBuff, long long iUserOff, int iUserSize)
       {
          bytes_read = rc;
          TRACEF(Error, "File::Read() failed read from disk");
-         // AMT commented line below should not be an immediate return, can have block refcount increased and map increased
-         // return rc;
+         return -1;
       }
    }
 
@@ -692,8 +690,6 @@ int File::Read(char* iUserBuff, long long iUserOff, int iUserSize)
    {
       XrdSysCondVarHelper _lck(m_downloadCond);
 
-      // AMT what is stamp block ???
-
       // blks_to_process can be non-empty, if we're exiting with an error.
       std::copy(blks_to_process.begin(), blks_to_process.end(), std::back_inserter(blks_processed));
 
@@ -759,7 +755,7 @@ void File::WriteBlockToDisk(Block* b)
       m_cfi.SetBitWritten(pfIdx);
 
       if (b->m_prefetch)
-         m_cfi.SetBitPrefetch();
+         m_cfi.SetBitPrefetch(pfIdx);
 
       // clLog()->Dump(XrdCl::AppMsg, "File::WriteToDisk() dec_ref_count %d %s", pfIdx, lPath());
       dec_ref_count(b);
@@ -830,7 +826,7 @@ void File::dec_ref_count(Block* b)
    b->m_refcnt--;
    assert(b->m_refcnt >= 0);
 
-   //AMT ... this is ugly, ... File::Read() can decrease ref count before waiting to be , prefetch starts with refcnt 0
+   // File::Read() can decrease ref count before waiting to be , prefetch starts with refcnt 0
    if (b->m_refcnt == 0 && b->is_finished())
    {
       free_block(b);
@@ -876,14 +872,10 @@ void File::ProcessBlockResponse(Block* b, int res)
    }
    else
    {
-      // AMT how long to keep?
-      // when to retry?
+      // TODO: how long to keep? when to retry?
       TRACEF(Error, "File::ProcessBlockResponse block " << b << "  " << (int)(b->m_offset/BufferSize()) << " error=" << res);
       // XrdPosixMap::Result(*status);
-      // AMT could notfiy global cache we dont need RAM for that block
       b->set_error_and_free(res);
-
-      // ??? AMT how long to keep
       inc_ref_count(b);
    }