[ https://issues.apache.org/jira/browse/HDFS-13762?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16772797#comment-16772797 ]
Uma Maheswara Rao G commented on HDFS-13762: -------------------------------------------- Thanks [~PhiloHe] for updating the patch. Please find the following feedback. I am still reviewing it and will post remaining comments tomorrow. # {code:java} this.pmemManager = new PmemVolumeManager(); this.pmemManager.load(pmemVolumes); PmemMappedBlock.setPersistentMemoryManager(this.pmemManager); PmemMappedBlock.setDataset(dataset); {code} I think interface design can be cleaner. Now we have interfaces for MappableBlock but we really will not depend much on interface, we actually directly use class to load block as they are staic methods. I am thinking something like this: Lets have MemoryVolumeManager interface which will be implemented by two separate impelemnattions: PMemVolumeManager and MemMappedVolumeManager They can have init methods and getMappableBlockLoader They can return respective MappableBlockLoader classes, they are PMemMappedBlock or MemoryMappedBlock Then this loader will have interfaces of loadBlock, getLength, and afterCache. Lets make current static load methods can be provate static methods in PMemMappedBlock and MemoryMappedBlock and use them in loadBlock interface implemenattion. Does this make sense to you? This will avoid the if check every time at {code:java} if (pmemManager == null) { mappableBlock = MemoryMappedBlock.load(length, blockIn, metaIn, blockFileName, key); } else { mappableBlock = PmemMappedBlock.load(length, blockIn, metaIn, blockFileName, key); } {code} MemoryVolumeManager#getMappableBlockLoader will get right loader and load the block. # "Fail to msync region. --> Failed to msync region # // Load Intel ISA-L : you may want to removed this in pmdk file. I saw in couple of other places referring to ISA-L. Please remove them as its not related to that. # If some DNs configured with pmem volumes and some not, then blocks will be cached in both mmapped and pmemmapped? Probably you would like to document your recommendation? # You may want to update the documentation for this feature https://hadoop.apache.org/docs/current/hadoop-project-dist/hadoop-hdfs/CentralizedCacheManagement.html # Since its persistet, how does this handle or remember cached blocks when DN restart? # Do you want to have different metrics for different cache types? I am not sure its really necessary, but people may be interested to know how many blocks cached in pmem area? (Low priority) # {noformat} Build FAILED. "C:\Users\umgangum\Work\hadoop\hadoop-common-project\hadoop-common\src\main\native\native.sln" (default target) (1) -> "C:\Users\umgangum\Work\hadoop\hadoop-common-project\hadoop-common\src\main\native\native.vcxproj" (default target) (2) -> (ClCompile target) -> src\org\apache\hadoop\util\bulk_crc32.c(59): warning C4091: 'static ' : ignored on left of 'int' when no variable is declared [C:\Users\umgangum\Work\hadoop\hadoop-common-project\hadoop-common\src\main\native\native.vcxproj] "C:\Users\umgangum\Work\hadoop\hadoop-common-project\hadoop-common\src\main\native\native.sln" (default target) (1) -> "C:\Users\umgangum\Work\hadoop\hadoop-common-project\hadoop-common\src\main\native\native.vcxproj" (default target) (2) -> (ClCompile target) -> c:\users\umgangum\work\hadoop\hadoop-common-project\hadoop-common\src\main\native\src\org\apache\hadoop\io\nativeio\pmdk_load.h(52): error C2061: syntax error : identifier '__d_pmem_map_file' [C:\Users\umgangum\Work\hadoop\hadoop-common-project\hadoop-common\src\main\native\native.vcxproj] {noformat} You need to fix for Windows as well. Am I missing something? # From pmdk_load.h {code} __d_pmem_map_file pmem_map_file; __d_pmem_unmap pmem_unmap; __d_pmem_is_pmem pmem_is_pmem; __d_pmem_drain pmem_drain; __d_pmem_memcpy_nodrain pmem_memcpy_nodrain; __d_pmem_msync pmem_msync; {code} Quick question does this types available if non-unix env? I think you did tydef in Unix env only right > Support non-volatile storage class memory(SCM) in HDFS cache directives > ----------------------------------------------------------------------- > > Key: HDFS-13762 > URL: https://issues.apache.org/jira/browse/HDFS-13762 > Project: Hadoop HDFS > Issue Type: Improvement > Components: caching, datanode > Reporter: Sammi Chen > Assignee: Feilong He > Priority: Major > Attachments: HDFS-13762.000.patch, HDFS-13762.001.patch, > HDFS-13762.002.patch, HDFS-13762.003.patch, HDFS-13762.004.patch, > HDFS-13762.005.patch, HDFS-13762.006.patch, HDFS-13762.007.patch, > HDFS-13762.008.patch, SCMCacheDesign-2018-11-08.pdf, SCMCacheTestPlan.pdf > > > No-volatile storage class memory is a type of memory that can keep the data > content after power failure or between the power cycle. Non-volatile storage > class memory device usually has near access speed as memory DIMM while has > lower cost than memory. So today It is usually used as a supplement to > memory to hold long tern persistent data, such as data in cache. > Currently in HDFS, we have OS page cache backed read only cache and RAMDISK > based lazy write cache. Non-volatile memory suits for both these functions. > This Jira aims to enable storage class memory first in read cache. Although > storage class memory has non-volatile characteristics, to keep the same > behavior as current read only cache, we don't use its persistent > characteristics currently. > > > -- This message was sent by Atlassian JIRA (v7.6.3#76005) --------------------------------------------------------------------- To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org