Apache Hadoop代码质量：生产VS测试

为了获得高质量的生产代码，仅确保测试的最大覆盖范围还不够。无疑，出色的结果需要主要的项目代码和测试才能有效地协同工作。因此，测试必须与源代码一样受到重视。体面的测试是成功的关键因素，因为它将赶上生产的衰退。让我们看一下PVS-Studio静态分析器警告，以查看测试错误并不比生产错误更严重这一事实的重要性。当今的焦点：Apache Hadoop。 

关于该项目

那些以前对大数据感兴趣的人可能已经听说过Apache Hadoop项目或与之合作。简而言之，Hadoop是可以用作构建和使用大数据系统的基础的框架。

Hadoop由四个主要模块组成;他们每个人都执行Big Dat $$ anonymous $$ nalytics系统所需的特定任务：

• Hadoop通用。
• MapReduce。
• Hadoop分布式文件系统。
• YARN。

关于支票

如文档中所示，PVS-Studio可以通过多种方式集成到项目中：

• 使用Maven插件。
• 使用Gradle插件。
• 使用Gradle IntellJ IDEA。
• 直接使用分析仪。

Hadoop基于Maven构建系统;因此，检查没有任何障碍。

在集成了文档中的脚本并编辑了一个pom.xml文件(依赖项中有一些模块不可用)之后，分析开始了!

分析完成后，我选择了最有趣的警告，并注意到在生产代码和测试中，我具有相同数量的警告。通常，我不考虑测试中的分析器警告。但是，当我将它们分开时，我无法不理会“测试”警告。我想：“为什么不看一下它们，因为测试中的错误也可能带来不利的后果。” 它们可能导致错误或部分测试，甚至导致杂乱。

选择最有趣的警告后，我将其分为以下几类：生产，测试和四个主要Hadoop模块。现在，我很高兴对分析仪警告进行回顾。

生产代码

Hadoop常见

V6033已经添加了具有相同键“ KDC_BIND_ADDRESS”的项目。MiniKdc.java(163)，MiniKdc.java(162)

Java

1个公共 类 MiniKdc {2  ....3 私有 静态 最终 Set < String >  
 PROPERTIES  =  new  HashSet < String >（）;4  ....5 静态 {6  性质。 
添加（ORG_NAME）;7  性质。添加（ORG_DOMAIN）;8  性质。添加 
（KDC_BIND_ADDRESS）;9  性质。添加（KDC_BIND_ADDRESS）; // <=10  性 
质。加（KDC_PORT）;11  性质。加（INSTANCE）;12   ....13  }14  ....15} 

HashSet检查项目时，a中两次增加的值 是一个非常常见的缺陷。第二个添加项将被忽略。我希望这种重复只是一场不必要的悲剧。如果要添加另一个值怎么办?

MapReduce

V6072找到两个相似的代码片段。也许这是一个错字， localFiles应该使用变量代替 localArchives。

• LocalDistributedCacheManager.java(183)。
• LocalDistributedCacheManager.java(178)。
• LocalDistributedCacheManager.java(176)。
• LocalDistributedCacheManager.java(181)。

Java

1个公共 同步 无效 设置（JobConf  conf，JobID  jobId）抛出 IOException  
{2  ....3 //使用本地化数据更新配置对象。4 如果（！localArchives。的 
isEmpty（））{5  conf。集（MRJobConfig。 
CACHE_LOCALARCHIVES，StringUtils的6     。 
arrayToString（localArchives。指定者（新 字符串 [ localArchives  //  
<=7       。大小（）]）））；8  }9 如果（！localFiles。的 
isEmpty（））{10  conf。集（MRJobConfig。 
CACHE_LOCALFILES，StringUtils的11     。arrayToString（localFiles。指 
定者（新 字符串 [ localArchives    // <=12       。大小 
（）]）））；13  }14  ....15} 

V6072诊断程序有时会产生一些有趣的发现。此诊断的目的是检测由于复制粘贴和替换两个变量而导致的相同类型的代码片段。在这种情况下，某些变量甚至保持“不变”。

上面的代码演示了这一点。在第一个块中， localArchives变量用于第二个类似的片段 localFiles。如果您认真研究此代码，而不是很快进行遍历，这通常在代码审阅时发生，那么您会注意到该片段，作者忘记了替换该 localArchives变量。

这种失态可能导致以下情况：

• 假设我们有localArchives(大小= 4)和localFiles(大小= 2)。
• 创建数组时 localFiles.toArray(new String[localArchives.size()])，最后两个元素将为null(["pathToFile1", "pathToFile2", null, null])。
• 然后， org.apache.hadoop.util.StringUtils.arrayToString将返回数组的字符串表示形式，其中最后的文件名将显示为“ null”(“ pathToFile1，pathToFile2，null，null”)。

所有这些都将进一步传递，上帝只知道这种情况下会有什么样的检查。

V6007表达式'children.size()> 0'始终为true。Queue.java(347)

Java

1个boolean  isHierarchySameAs（Queue  newState）{2  ....3 如果（孩子  
==  空 ||  孩子。大小（）==  0）{4   ....5  }6 否则 ，如果（孩子。大 
小（）>  0）7  {8   ....9  }10  ....11} 

由于要单独检查元素数是否为0，因此进一步检查children.size()> 0将始终为true。

HDFS

V6001在'%'运算符的左侧和右侧有相同的子表达式'this.bucketSize'。RollingWindow.java(79)。

Java

1个 RollingWindow（int  windowLenMs，int  numBuckets）{2  buckets  =  
 new  Bucket [ numBuckets ];3  for（int  i  =  0 ; i  <  numBuckets ;  
i ++）{4   buckets [ i ] =  new  Bucket（）;5   }6  这个。windowLenMs  
 =  windowLenMs ;7  这个。bucketSize  =  windowLenMs  /  numBuckets  
;8  如果（此。bucketSize  ％ bucketSize  ！=  0）{      // <=9   抛出 
 新的 IllegalArgumentException（10     “滚动窗口中的存储桶大小不是整 
数：windowLenMs =”11       +  windowLenMs  +  “ numBuckets =”“  + 
  numBuckets）;12   }13  } 

YARN

V6067两个或更多案例分支执行相同的操作。TimelineEntityV2Converter.java(386)，TimelineEntityV2Converter.java(389)。

Java

1个 公共 静态 ApplicationReport2  
convertToApplicationReport（TimelineEntity  实体）3{4  ....5 如果 
（指标 ！=  null）{6  long  vcoreSeconds  =  0 ;7  long   
memorySeconds  =  0 ;8  long  preemptedVcoreSeconds  =  0 ;9  long 
  preemptedMemorySeconds  =  0 ;1011  对于（TimelineMetric  指标： 
指标）{12   开关（度量。的getId（））{13   case   
ApplicationMetricsConstants。APP_CPU_METRICS：14    vcoreSeconds  
 =  getAverageValue（度量。的GetValues。（）的值（））;15    休息  
;16   case  ApplicationMetricsConstants。APP_MEM_METRICS：17    
 memorySeconds  = ....;18岁    休息 ;19   case  
 ApplicationMetricsConstants。APP_MEM_PREEMPT_METRICS：20     
preemptedVcoreSeconds  = ....;             // <=21    休息 ;22    
case  ApplicationMetricsConstants。APP_CPU_PREEMPT_METRICS：23   
  preemptedVcoreSeconds  = ....;             // <=24    休息 ;25  
  默认值：26    //不应该发生27    休息 ;28    }29   }30   ....31 
  }32  ....33} 

相同的代码片段位于两个case分支中。到处都是!在大多数情况下，这不是真正的错误，而只是考虑重构switch语句的原因。对于当前的情况，情况并非如此。重复的代码片段设置变量的值preemptedVcoreSeconds。如果仔细查看所有变量和常量的名称，可能会得出结论，在这种情况下， if metric.getId() == APP_MEM_PREEMPT_METRICS必须为preemptedMemorySeconds变量设置 值，而不是 preemptedVcoreSeconds。在这方面，在switch语句之后，preemptedMemorySeconds将始终保持0，而的值 preemptedVcoreSeconds可能不正确。

V6046格式错误。期望使用不同数量的格式项。不使用的参数：2. AbstractSchedulerPlanFollower.java(186)

Java

1个@Override2市民 同步 无效 synchronizePlan（规划 计划，布尔  
shouldReplan）3{4  ....5 尝试6  {7   
setQueueEntitlement（planQueueName，....）;8  }9 捕获（YarnException  
 e）10  {11  LOG。警告（“尝试为计划{{}确定保留大小时发生异常”，12 
       currResId，13       planQueueName，14       e）;15  }16  ....17} 

planQueueName记录时不使用该 变量。在这种情况下，要么复制太多，要么格式字符串未完成。但是我仍然要责备旧的复制粘贴，在某些情况下，将其粘贴在脚上真是太好了。

测试代码

Hadoop常见

V6072找到两个相似的代码片段。也许这是一个错字，应该使用'allSecretsB'变量而不是'allSecretsA'。

TestZKSignerSecretProvider.java(316)，TestZKSignerSecretProvider.java(309)，TestZKSignerSecretProvider.java(306)，TestZKSignerSecretProvider.java(313)。

Java

1个public  void  testMultiple（整数 顺序）引发 异常 {2   ....3   
currentSecretA  =  secretProviderA。getCurrentSecret（）;4   
allSecretsA  =  secretProviderA。getAllSecrets（）;5  断言。 
assertArrayEquals（secretA2，currentSecretA）;6  断言。的 
assertEquals（2，allSecretsA。长度）;     // <=7  断言。 
assertArrayEquals（secretA2，allSecretsA [ 0 ]）;8  断言。 
assertArrayEquals（secretA1，allSecretsA [ 1 ]）;910  currentSecretB   
=  secretProviderB。getCurrentSecret（）;11  allSecretsB  =   
secretProviderB。getAllSecrets（）;12  断言。 
assertArrayEquals（secretA2，currentSecretB）;13  断言。的 
assertEquals（2，allSecretsA。长度）;    // <=14  断言。 
assertArrayEquals（secretA2，allSecretsB [ 0 ]）;15  断言。 
assertArrayEquals（secretA1，allSecretsB [ 1 ]）;16   ....17} 

再次是V6072。仔细观察变量allSecretsA和allSecretsB。

V6043考虑检查“ for”运算符。迭代器的初始值和最终值相同。TestTFile.java(235)。

Java

1个私有 int  readPrepWithUnknownLength（扫描 仪扫描仪，int   
start，int  n）2  引发 IOException {3 对于（int  i  =  start ; i  <   
start ; i ++）{4  字符串 键 =  字符串。格式（localFormatter，i）;5   
字节 [] 读取 =  readKey（扫描仪）;6  assertTrue（“键不等于”，阵列。 
等号（键。的getBytes（），读））;7  尝试 {8   读取 =  读取值（扫描 
仪）;9   assertTrue（false）;10   }11  抓（IOException  即）{12   // 
应该抛出异常13   }14  字符串 值 =  “值”  +  键；15  读取 =   
readLongValue（扫描器，值。的getBytes（）。长度）;16  assertTrue（“n 
要相等的值”，阵列。等号（读，值。的getBytes（）））;17  扫描仪。前进 
（）;18岁  }19 返回（start  +  n）;20} 

始终是绿色的测试?=)。循环的一部分，即测试本身的一部分，将永远不会执行。这是由于以下事实：for语句中的初始计数器值和最终计数器值相等 。结果，条件 i < start将立即变为假，从而导致这种行为。我浏览了测试文件，然后得出i < (start + n)必须在循环条件下编写的结论 。

MapReduce

V6007表达式'byteAm <0'始终为false。DataWriter.java(322)

Java

个GenerateOutput  writeSegment（long  byteAm，OutputStream  out）2  
 引发 IOException {3 long  headerLen  =  getHeaderLength（）;4  
if（byteAm  <  headerLen）{5  //没有足够的字节写头6  返回 新  
GenerateOutput（0，0）;7  }8 //调整标题长度9 byteAm-  =  headerLen  
;10 if（byteAm  <  0）{   // <=11  byteAm  =  0 ;12  }13  ....14} 

条件 byteAm < 0始终为假。为了弄清楚，让我们在上面的代码再看一遍。如果测试执行达到了操作的要求 byteAm -= headerLen，则意味着 byteAm >= headerLen。从这里开始，减去后，该 byteAm值将永远不会为负。那就是我们必须证明的。

HDFS

V6072找到两个相似的代码片段。也许这是一个错字， normalFile应该使用变量代替 normalDir。TestWebHDFS.java(625)，TestWebHDFS.java(615)，TestWebHDFS.java(614)，TestWebHDFS.java(624)

Java

1个public  void  testWebHdfsErasureCodingFiles（）引发 异常 {2  
 ....3 最终 路径 normalDir  =  新 路径（“ / dir”）;4 dfs。 
mkdirs（normalDir）;5 最终 路径 normalFile  =  新 路径 
（normalDir，“ file.log ”）;6  ....7 //逻辑块＃18 时间filestatus  
 expectedNormalDirStatus  =  DFS。getFileStatus（normalDir）;9  
FileStatus  actualNormalDirStatus  =  webHdfs。 
getFileStatus（normalDir）; // <=10 断言。的 
assertEquals（expectedNormalDirStatus。isErasureCoded（），11    
        actualNormalDirStatus。isErasureCoded（））;12  
ContractTestUtils。assertNotErasureCoded（dfs，normalDir）;13  
assertTrue（normalDir  +  “应具有擦除编码中未设置”  +  
....）;1415 //逻辑块＃216 时间filestatus  expectedNormalFileStatus  
 =  DFS。getFileStatus（normalFile）;17 FileStatus   
actualNormalFileStatus  =  webHdfs。getFileStatus（normalDir）; // 
 <=18岁 断言。的assertEquals（expectedNormalFileStatus。 
isErasureCoded（），19           actualNormalFileStatus。 
isErasureCoded（））;20 ContractTestUtils。 
assertNotErasureCoded（dfs，normalFile）;21 assertTrue（normalFile 
  +  “应具有擦除编码中未设置”  + ....）;22} 

信不信由你，它又是V6072!只需跟随变量 normalDir和 normalFile。

V6027通过调用同一函数来初始化变量。可能是错误或未优化的代码。TestDFSAdmin.java(883)，TestDFSAdmin.java(879)。

Java

1个私人 void  verifyNodesAndCorruptBlocks（2  最终 整数 numDn，3  
 final  int  numLiveDn，4  final  int  numCorruptBlocks，5  final   
int  numCorruptECBlockGroups，6  最终的 DFSClient  客户端，7  最后的  
long  最高PriorityLowRedundancyReplicatedBlocks，8  最后的 Long  最高 
PriorityLowRedundancyECBlocks）9  引发 IOException10{11 / *初始化变 
量* /12  ....13 最后的 字符串 ExpectedCorruptedECBlockGroupsStr  =  
 String。格式（14   “具有损坏的内部块的块组：％d”，15    
numCorruptECBlockGroups）;16 最后的 字符串的  
highestPriorityLowRedundancyReplicatedBlocksStr17   =  字符串。格式 
（18岁   “ \ t具有最高优先级的低冗余块”  +19     “要恢 
复：％d”，20   maximumPriorityLowRedundancyReplicatedBlocks）;21 最 
后的 字符串 highestPriorityLowRedundancyECBlocksStr  =  String。格式 
（22   “ \ t具有最高优先级的低冗余块”  +23     “要恢复：％d”，24   
 maximumPriorityLowRedundancyReplicatedBlocks）;25  ....26} 

在这个片段中，highestPriorityLowRedundancyReplicatedBlocksStr和highestPriorityLowRedundancyECBlocksStr用相同的值初始化。通常应该是这样，但事实并非如此。变量的名称又长又相似，因此复制粘贴的片段没有进行任何更改也就不足为奇了。为了解决这个问题，在初始化highestPriorityLowRedundancyECBlocksStr变量时，作者必须使用输入参数highestPriorityLowRedundancyECBlocks。此外，最有可能的是，他们仍然需要更正格式行。

V6019检测不到代码。可能存在错误。TestReplaceDatanodeFailureReplication.java(222)。

Java

1个私人 虚空2verifyFileContent（....，SlowWriter [] slowwriters）引发 
 IOException3{4 LOG。信息（“验证文件”）；5 对（INT  我 =  0 ; 我 <  
 slowwriters。长度 ; 我++）{6  LOG。信息（slowwriters [ 我 ]。文件路 
径 + ....）;7  FSDataInputStream  in  =  null ;8  尝试 {9   in  =   
fs。开放（slowwriters [ 我 ]。文件路径）;10   for（int  j  =  0，x ;;  
j ++）{11    x  =  in中。阅读（）;12    如果（（x）！=  - 1）{13    
  断言。assertEquals（j，x）;14     } 其他 {15     回报 ;16     }17 
    }18岁   } 最后 {19   IOUtils。closeStream（in）;20   }21  }22} 

分析仪抱怨i++无法更改循环中的计数器。这意味着在for (int i = 0; i < slowwriters.length; i++) {....}循环中最多将执行一次迭代。让我们找出原因。在第一次迭代中，我们将线程与对应的文件链接起来，以slowwriters[0]供进一步阅读。接下来，我们通过loop读取文件内容for (int j = 0, x;; j++)。

如果我们读取了一些相关的内容，我们会将读取的字节与j计数器的当前值进行比较assertEquals(如果检查不成功，则测试失败)。

如果文件检查成功，并且到达文件末尾(读取为-1)，则该方法退出。

因此，无论在检查期间发生什么 slowwriters[0]，都不会去检查后续元素。最有可能的是break，必须使用a代替return。

YARN

V6019检测不到代码。可能存在错误。TestNodeManager.java(176)

Java

1个@测试2公共 无效 3testCreationOfNodeLabelsProviderService（）引发 
 InterruptedException {4 尝试 {5   ....6  } catch（异常 e）{7  断言。 
失败（“捕获到异常”）；8  e。printStackTrace（）;9  }10} 

在这种情况下，该 Assert.fail方法将中断测试，并且在发生异常的情况下不会打印堆栈跟踪。如果有关捕获到的异常的消息在这里足够多，则最好删除打印堆栈跟踪的记录，以免造成混淆。如果需要打印，则只需交换它们。

已发现许多类似的片段：

• V6019检测不到代码。可能存在错误。TestResourceTrackerService.java(928)。
• V6019检测不到代码。可能存在错误。TestResourceTrackerService.java(737)。
• V6019检测不到代码。可能存在错误。TestResourceTrackerService.java(685)。

V6072找到两个相似的代码片段。也许这是一个错字， publicCache应该使用变量代替 usercache。

 

TestResourceLocalizationService.java(315)，  
TestResourceLocalizationService.java(309)，  
TestResourceLocalizationService.java(307)，  
TestResourceLocalizationService.java(313)  
Java 

1个@测试2公共 无效 testDirectoryCleanupOnNewlyCreatedStateStore（）3  
 抛出 IOException，URISyntaxException4{5  ....6 //验证目录创建7 对于 
（路径 p：localDirs）{8  p  =  新 路径（（新 URI（p。的 
toString（）））。的getPath（））;910  //逻辑块＃111  路径 usercache 
  =  新 路径（p，ContainerLocalizer。USERCACHE）;12  验证（spylfs）。 
重命名（当量（usercache），任何（路径。类），任何（））; // <=13  验 
证（spylfs）。mkdir（eq（usercache），....）;1415  //逻辑块＃216  路 
径 publicCache  =  新 路径（p，ContainerLocalizer。FILECACHE）;17  验 
证（spylfs）。重命名（当量（usercache），任何（路径。类），任何 
（））; // <=18岁  验证（spylfs）。 
mkdir（eq（publicCache），....）;19   ....20  }21  ....22} 

最后，再次是V6072 =)。用于查看可疑片段的变量： usercache 和publicCache。

结论

开发中编写了成千上万行代码。生产代码通常保持清洁，没有错误，缺陷和缺陷(开发人员测试他们的代码，检查代码等)。在这方面，测试肯定不如。测试中的缺陷很容易隐藏在“绿色刻度”后面。正如您可能从今天的警告回顾中了解到的那样，绿色测试并不总是一项成功的检查。

这次，当检查Apache Hadoop代码库时，在生产代码和测试中都非常需要静态分析，而静态分析在开发中也起着重要作用。因此，如果您关心代码和测试质量，建议您着眼于静态分析。