Merge branch 'release-1.16.7' into stable

pfsagentd/README.md

@@ -54,6 +54,7 @@ SharedFileLimit:                                           1000
 ExclusiveFileLimit:                                         100
 DirtyFileLimit:                                              50
 DirtyLogSegmentLimit:                                        50
+ExtentMapEntryLimit:                                    1048576
 MaxFlushSize:                                          10485760
 MaxFlushTime:                                             200ms
 LogFilePath:                             /var/log/pfsagentd.log

pfsagentd/file_inode.go

@@ -409,7 +409,8 @@ func (chunkedPutContext *chunkedPutContextStruct) complete() {
 
 	err = globals.retryRPCClient.Send("RpcWrote", wroteRequest, wroteReply)
 	if nil != err {
-		logFatalf("*chunkedPutContextStruct.complete() failed Server.RpcWrote: %v", err)
+		// logFatalf("*chunkedPutContextStruct.complete() failed Server.RpcWrote: %v", err)
+		logWarnf("TODO (i.e. convert to logFatalf) *chunkedPutContextStruct.complete() failed Server.RpcWrote: %v", err)
 	}
 
 	// Remove this chunkedPutContext from fileInode.chunkedPutList and mark as Done()
@@ -515,16 +516,12 @@ func (fileInode *fileInodeStruct) populateExtentMap(fileOffset uint64, length ui
 
 	if nil == fileInode.extentMap {
 		// Create an empty ExtentMap... and perform initial population
-		// This counts as a reference, too
-
-		fileInode.reference()
 
 		fileInode.extentMap = sortedmap.NewLLRBTree(sortedmap.CompareUint64, fileInode)
 
 		err = fileInode.populateExtentMapHelper(fileOffset)
 		if nil != err {
 			fileInode.extentMap = nil
-			fileInode.dereference()
 			return
 		}
 	}
@@ -566,7 +563,6 @@ Restart:
 			err = fileInode.populateExtentMapHelper(curFileOffset)
 			if nil != err {
 				fileInode.extentMap = nil
-				fileInode.dereference()
 				return
 			}
 			goto Restart
@@ -583,7 +579,6 @@ Restart:
 			err = fileInode.populateExtentMapHelper(curFileOffset)
 			if nil != err {
 				fileInode.extentMap = nil
-				fileInode.dereference()
 				return
 			}
 			goto Restart
@@ -596,7 +591,6 @@ Restart:
 			err = fileInode.populateExtentMapHelper(curExtent.fileOffset + curExtent.length)
 			if nil != err {
 				fileInode.extentMap = nil
-				fileInode.dereference()
 				return
 			}
 			goto Restart
@@ -768,9 +762,7 @@ func (fileInode *fileInodeStruct) updateExtentMap(newExtent *multiObjectExtentSt
 	)
 
 	if nil == fileInode.extentMap {
-		// Create an empty ExtentMap... This counts as a reference, too
-
-		fileInode.reference()
+		// Create an empty ExtentMap
 
 		fileInode.extentMap = sortedmap.NewLLRBTree(sortedmap.CompareUint64, fileInode)
 	}

pfsagentd/globals.go

@@ -47,6 +47,7 @@ type configStruct struct {
 	ExclusiveFileLimit           uint64
 	DirtyFileLimit               uint64
 	DirtyLogSegmentLimit         uint64
+	ExtentMapEntryLimit          uint64
 	MaxFlushSize                 uint64
 	MaxFlushTime                 time.Duration
 	LogFilePath                  string // Unless starting with '/', relative to $CWD; == "" means disabled
@@ -193,11 +194,12 @@ type fileInodeStruct struct {
 	//                                                     fileInodeLeaseStateSharedPromoting
 	//                                                     fileInodeLeaseStateExclusiveRequested
 	//                                                     fileInodeLeaseStateExclusiveGranted
-	extentMapFileSize         uint64             //        FileSize covered by .extentMap (.chunkedPutList may extend)
-	extentMap                 sortedmap.LLRBTree //    Key == multiObjectExtentStruct.fileOffset; Value == *multiObjectExtentStruct
-	chunkedPutList            *list.List         //    FIFO List of chunkedPutContextStruct's
-	flushInProgress           bool               //    Serializes (& singularizes) explicit Flush requests
-	chunkedPutFlushWaiterList *list.List         //    List of *sync.WaitGroup's for those awaiting an explicit Flush
+	extentMapFileSize            uint64             // FileSize covered by .extentMap (.chunkedPutList may extend)
+	extentMap                    sortedmap.LLRBTree // Key == multiObjectExtentStruct.fileOffset; Value == *multiObjectExtentStruct
+	extentMapLenWhenUnreferenced int                // Each time references drops to zero, update globals.extentMapEntriesCached
+	chunkedPutList               *list.List         // FIFO List of chunkedPutContextStruct's
+	flushInProgress              bool               // Serializes (& singularizes) explicit Flush requests
+	chunkedPutFlushWaiterList    *list.List         // List of *sync.WaitGroup's for those awaiting an explicit Flush
 	//                                                   Note: These waiters cannot be holding fileInodeStruct.Lock
 	dirtyListElement *list.Element //                  Element on globals.fileInodeDirtyList (or nil)
 }
@@ -362,6 +364,7 @@ type globalsStruct struct {
 	lastFH                          uint64               // Valid FH's start at 1
 	logSegmentCacheMap              map[logSegmentCacheElementKeyStruct]*logSegmentCacheElementStruct
 	logSegmentCacheLRU              *list.List // Front() is oldest logSegmentCacheElementStruct.cacheLRUElement
+	extentMapEntriesCached          int        // Running total of all fileInode.extentMapLenWhenUnreferenced values
 	metrics                         *metricsStruct
 	stats                           *statsStruct
 }
@@ -516,6 +519,11 @@ func initializeGlobals(confMap conf.ConfMap) {
 		logFatal(err)
 	}
 
+	globals.config.ExtentMapEntryLimit, err = confMap.FetchOptionValueUint64("Agent", "ExtentMapEntryLimit")
+	if nil != err {
+		logFatal(err)
+	}
+
 	globals.config.MaxFlushSize, err = confMap.FetchOptionValueUint64("Agent", "MaxFlushSize")
 	if nil != err {
 		logFatal(err)
@@ -701,6 +709,8 @@ func initializeGlobals(confMap conf.ConfMap) {
 	globals.logSegmentCacheMap = make(map[logSegmentCacheElementKeyStruct]*logSegmentCacheElementStruct)
 	globals.logSegmentCacheLRU = list.New()
 
+	globals.extentMapEntriesCached = 0
+
 	globals.metrics = &metricsStruct{}
 	globals.stats = &statsStruct{}
 
@@ -757,6 +767,7 @@ func uninitializeGlobals() {
 	globals.lastFH = 0
 	globals.logSegmentCacheMap = nil
 	globals.logSegmentCacheLRU = nil
+	globals.extentMapEntriesCached = 0
 	globals.metrics = nil
 	globals.stats = nil
 }

pfsagentd/inode.go

@@ -18,19 +18,14 @@ import (
 // and the globals.fileInodeMap must, therefore, never "forget" a fileInodeStruct
 // for which a reference is still available.
 //
-// References occur in three cases:
+// References occur in two cases:
 //
 //   A Shared or Exclusive Lock is being requested or held for a FileInode:
 //
 //     The lock requestor must first reference a FileInode before makeing
 //     the Shared or Exclusive Lock request. After releasing the Lock, they
 //     dereference it.
 //
-//   A FileInode's ExtentMap is being fetched or maintained:
-//
-//     In this case, a single reference is made to indicate that this instance
-//     is caching the FileInode's size and some or all of its ExtentMap.
-//
 //   A FileInode has one or more in-flight LogSegment PUTs underway:
 //
 //     In this case, each LogSegment PUT is represented by a reference from the
@@ -59,19 +54,20 @@ func referenceFileInode(inodeNumber inode.InodeNumber) (fileInode *fileInodeStru
 		fileInode.references++
 	} else {
 		fileInode = &fileInodeStruct{
-			InodeNumber:               inodeNumber,
-			cachedStat:                nil,
-			references:                1,
-			leaseState:                fileInodeLeaseStateNone,
-			sharedLockHolders:         list.New(),
-			exclusiveLockHolder:       nil,
-			lockWaiters:               list.New(),
-			extentMapFileSize:         0,
-			extentMap:                 nil,
-			chunkedPutList:            list.New(),
-			flushInProgress:           false,
-			chunkedPutFlushWaiterList: list.New(),
-			dirtyListElement:          nil,
+			InodeNumber:                  inodeNumber,
+			cachedStat:                   nil,
+			references:                   1,
+			leaseState:                   fileInodeLeaseStateNone,
+			sharedLockHolders:            list.New(),
+			exclusiveLockHolder:          nil,
+			lockWaiters:                  list.New(),
+			extentMapFileSize:            0,
+			extentMap:                    nil,
+			extentMapLenWhenUnreferenced: 0,
+			chunkedPutList:               list.New(),
+			flushInProgress:              false,
+			chunkedPutFlushWaiterList:    list.New(),
+			dirtyListElement:             nil,
 		}
 
 		fileInode.cacheLRUElement = globals.unleasedFileInodeCacheLRU.PushBack(fileInode)
@@ -105,6 +101,8 @@ func (fileInode *fileInodeStruct) reference() {
 func (fileInode *fileInodeStruct) dereference() {
 	var (
 		delayedLeaseRequestList *list.List
+		err                     error
+		extentMapLen            int
 	)
 
 	delayedLeaseRequestList = nil
@@ -114,6 +112,18 @@ func (fileInode *fileInodeStruct) dereference() {
 	fileInode.references--
 
 	if 0 == fileInode.references {
+		if nil == fileInode.extentMap {
+			globals.extentMapEntriesCached -= fileInode.extentMapLenWhenUnreferenced
+			fileInode.extentMapLenWhenUnreferenced = 0
+		} else {
+			extentMapLen, err = fileInode.extentMap.Len()
+			if nil != err {
+				logFatalf("*fileInodeStruct.dereference() call to fileInode.extentMap.Len() failed: %v", err)
+			}
+			globals.extentMapEntriesCached += extentMapLen - fileInode.extentMapLenWhenUnreferenced
+			fileInode.extentMapLenWhenUnreferenced = extentMapLen
+		}
+
 		delayedLeaseRequestList = honorInodeCacheLimits()
 	}
 
@@ -130,17 +140,17 @@ func (fileInode *fileInodeStruct) dereference() {
 //
 func honorInodeCacheLimits() (delayedLeaseRequestList *list.List) {
 	var (
-		cacheLimitToEnforce      int
-		delayedLeaseRequest      *fileInodeLeaseRequestStruct
-		fileInode                *fileInodeStruct
-		fileInodeCacheLRUElement *list.Element
+		delayedLeaseRequest          *fileInodeLeaseRequestStruct
+		fileInode                    *fileInodeStruct
+		fileInodeCacheLRUElement     *list.Element
+		fileInodeCacheLimitToEnforce int
 	)
 
 	delayedLeaseRequestList = list.New()
 
-	cacheLimitToEnforce = int(globals.config.ExclusiveFileLimit)
+	fileInodeCacheLimitToEnforce = int(globals.config.ExclusiveFileLimit)
 
-	for globals.exclusiveLeaseFileInodeCacheLRU.Len() > cacheLimitToEnforce {
+	for globals.exclusiveLeaseFileInodeCacheLRU.Len() > fileInodeCacheLimitToEnforce {
 		fileInodeCacheLRUElement = globals.exclusiveLeaseFileInodeCacheLRU.Front()
 		fileInode = fileInodeCacheLRUElement.Value.(*fileInodeStruct)
 		if (0 < fileInode.references) || (fileInodeLeaseStateExclusiveGranted != fileInode.leaseState) {
@@ -156,16 +166,16 @@ func honorInodeCacheLimits() (delayedLeaseRequestList *list.List) {
 		fileInode.cacheLRUElement = globals.sharedLeaseFileInodeCacheLRU.PushBack(fileInode)
 	}
 
-	cacheLimitToEnforce = int(globals.config.SharedFileLimit)
+	fileInodeCacheLimitToEnforce = int(globals.config.SharedFileLimit)
 
 	if globals.exclusiveLeaseFileInodeCacheLRU.Len() > int(globals.config.ExclusiveFileLimit) {
-		cacheLimitToEnforce -= globals.exclusiveLeaseFileInodeCacheLRU.Len() - int(globals.config.ExclusiveFileLimit)
-		if 0 > cacheLimitToEnforce {
-			cacheLimitToEnforce = 0
+		fileInodeCacheLimitToEnforce -= globals.exclusiveLeaseFileInodeCacheLRU.Len() - int(globals.config.ExclusiveFileLimit)
+		if 0 > fileInodeCacheLimitToEnforce {
+			fileInodeCacheLimitToEnforce = 0
 		}
 	}
 
-	for globals.sharedLeaseFileInodeCacheLRU.Len() > cacheLimitToEnforce {
+	for globals.sharedLeaseFileInodeCacheLRU.Len() > fileInodeCacheLimitToEnforce {
 		fileInodeCacheLRUElement = globals.sharedLeaseFileInodeCacheLRU.Front()
 		fileInode = fileInodeCacheLRUElement.Value.(*fileInodeStruct)
 		if (0 < fileInode.references) || (fileInodeLeaseStateSharedGranted != fileInode.leaseState) {
@@ -181,13 +191,14 @@ func honorInodeCacheLimits() (delayedLeaseRequestList *list.List) {
 		fileInode.cacheLRUElement = globals.unleasedFileInodeCacheLRU.PushBack(fileInode)
 	}
 
-	cacheLimitToEnforce = int(globals.config.ExclusiveFileLimit) - globals.exclusiveLeaseFileInodeCacheLRU.Len()
-	cacheLimitToEnforce += int(globals.config.SharedFileLimit) - globals.sharedLeaseFileInodeCacheLRU.Len()
+	fileInodeCacheLimitToEnforce = int(globals.config.ExclusiveFileLimit) - globals.exclusiveLeaseFileInodeCacheLRU.Len()
+	fileInodeCacheLimitToEnforce += int(globals.config.SharedFileLimit) - globals.sharedLeaseFileInodeCacheLRU.Len()
 
-	for globals.unleasedFileInodeCacheLRU.Len() > cacheLimitToEnforce {
+	for (nil != globals.unleasedFileInodeCacheLRU.Front()) && ((globals.unleasedFileInodeCacheLRU.Len() > fileInodeCacheLimitToEnforce) || (globals.config.ExtentMapEntryLimit < uint64(globals.extentMapEntriesCached))) {
 		fileInodeCacheLRUElement = globals.unleasedFileInodeCacheLRU.Front()
 		fileInode = fileInodeCacheLRUElement.Value.(*fileInodeStruct)
 		globals.unleasedFileInodeCacheLRU.Remove(fileInodeCacheLRUElement)
+		globals.extentMapEntriesCached -= fileInode.extentMapLenWhenUnreferenced
 		delete(globals.fileInodeMap, fileInode.InodeNumber)
 	}
 

pfsagentd/pfsagent.conf

@@ -20,6 +20,7 @@ SharedFileLimit:                                           1000
 ExclusiveFileLimit:                                         100
 DirtyFileLimit:                                              50
 DirtyLogSegmentLimit:                                        50
+ExtentMapEntryLimit:                                    1048576
 MaxFlushSize:                                          10485760
 MaxFlushTime:                                             200ms
 LogFilePath:                             /var/log/pfsagentd.log

pfsagentd/setup_teardown_test.go

@@ -100,6 +100,7 @@ func testSetup(t *testing.T) {
 		"Agent.ExclusiveFileLimit=100",
 		"Agent.DirtyFileLimit=50",
 		"Agent.DirtyLogSegmentLimit=50",
+		"Agent.ExtentMapEntryLimit=1048576",
 		"Agent.MaxFlushSize=10485760",
 		"Agent.MaxFlushTime=10s",
 		"Agent.LogFilePath=",

release_notes.md

@@ -1,5 +1,17 @@
 # ProxyFS Release Notes
 
+## 1.16.7 (July 16, 2020)
+
+### Bug Fixes:
+
+Avoid PFSAgent crashes when the lack of leases means it doesn't know
+that the underlying file inode has been removed when it is lazily
+flushing write data to it.
+
+Provide a cap on remaining unbounded memory growth due to caching of
+extent maps for files in PFSAgent. The new tunable that limits this
+memory consumption is [Agent]ExtentMapEntryLimit.
+
 ## 1.16.6 (July 13, 2020)
 
 ### Bug Fixes: