Geekly articles weekly

Apache Hadoop Code Quality: uji VS produksi

Gambar 1

Untuk mendapatkan kode produksi berkualitas tinggi, tidak cukup hanya dengan memberikan cakupan pengujian maksimum. Tidak diragukan lagi, untuk mencapai hasil yang tinggi, kode proyek utama dan tes harus bekerja dalam tandem kohesif yang sempurna. Karena itu, Anda perlu memperhatikan tes sebanyak kode utama. Menulis tes yang baik adalah kunci untuk mendapatkan regresi dalam produksi. Untuk menunjukkan pentingnya fakta bahwa bug dalam pengujian tidak lebih buruk daripada dalam produksi, kami akan mempertimbangkan analisis peringatan berikutnya dari penganalisa statis PVS-Studio. Target: Apache Hadoop.

Tentang proyek


Mereka yang pernah tertarik dengan Big Data mungkin pernah mendengar atau bekerja dengan proyek seperti Apache Hadoop . Singkatnya, Hadoop adalah kerangka kerja yang dapat digunakan sebagai dasar untuk membangun dan bekerja dengan sistem Big Data.

Hadoop terdiri dari empat modul utama, yang masing-masing melakukan tugas khusus yang diperlukan untuk sistem analisis data besar:

  • Hadoop biasa
  • Kurangi peta
  • Sistem File Terdistribusi Hadoop (Sistem File Terdistribusi Hadoop)
  • Benang

Namun, ada banyak bahan untuk membiasakan diri dengan itu di Internet.

Tentang Verifikasi


Seperti yang ditunjukkan dalam dokumentasi , PVS-Studio dapat diintegrasikan ke dalam proyek dengan berbagai cara:

  • menggunakan plugin maven;
  • menggunakan plugin gradle;
  • Menggunakan IntellJ IDEA
  • menggunakan analisa secara langsung.

Hadoop dibangun berdasarkan sistem pembangunan pakar, sehingga tidak ada kesulitan dengan verifikasi.

Setelah mengintegrasikan skrip dari dokumentasi dan sedikit menyesuaikan salah satu pom.xml (ada modul dalam dependensi yang tidak ada), analisis berjalan!

Setelah analisis, memilih peringatan yang paling menarik, saya perhatikan bahwa saya memiliki jumlah peringatan yang sama baik dalam kode produksi dan tes. Biasanya, saya tidak mempertimbangkan pemicu analisa yang jatuh pada tes. Tapi, membaginya, saya tidak bisa melewatkan peringatan dari kategori 'tes' melewati perhatian saya. "Kenapa tidak?" Saya pikir, karena bug dalam tes juga memiliki konsekuensi. Mereka dapat menyebabkan pengujian yang salah atau sebagian, atau bahkan omong kosong (hanya untuk pertunjukan, sehingga mereka selalu hijau).

Jadi, setelah mengumpulkan peringatan yang paling menarik, membaginya dengan kode (produksi, pengujian) dan empat modul Hadoop utama, saya membawa kepada Anda sebuah analisis operasi penganalisa.

Kode produksi


Hadoop biasa


V6033 Item dengan kunci yang sama 'KDC_BIND_ADDRESS' telah ditambahkan. MiniKdc.java (163), MiniKdc.java (162)

public class MiniKdc { .... private static final Set<String> PROPERTIES = new HashSet<String>(); .... static { PROPERTIES.add(ORG_NAME); PROPERTIES.add(ORG_DOMAIN); PROPERTIES.add(KDC_BIND_ADDRESS); PROPERTIES.add(KDC_BIND_ADDRESS); // <= PROPERTIES.add(KDC_PORT); PROPERTIES.add(INSTANCE); .... } .... }

Nilai yang ditambahkan dua kali ke HashSet adalah cacat umum saat memeriksa proyek. Bahkan, penambahan kedua akan diabaikan. Nah, jika duplikasi ini adalah kecelakaan yang tidak masuk akal. Tetapi bagaimana jika itu benar-benar berarti menambah nilai lain?

Kurangi peta


V6072 Dua fragmen kode serupa ditemukan. Mungkin, ini adalah kesalahan ketik dan variabel 'localFiles' harus digunakan daripada 'localArchives'. LocalDistributedCacheManager.java (183), LocalDistributedCacheManager.java (178), LocalDistributedCacheManager.java (176), LocalDistributedCacheManager.java (181)

 public synchronized void setup(JobConf conf, JobID jobId) throws IOException { .... // Update the configuration object with localized data. if (!localArchives.isEmpty()) { conf.set(MRJobConfig.CACHE_LOCALARCHIVES, StringUtils .arrayToString(localArchives.toArray(new String[localArchives // <= .size()]))); } if (!localFiles.isEmpty()) { conf.set(MRJobConfig.CACHE_LOCALFILES, StringUtils .arrayToString(localFiles.toArray(new String[localArchives // <= .size()]))); } .... }

Diagnostik V6072 terkadang menghasilkan temuan yang sangat menarik. Inti dari diagnostik adalah mencari jenis fragmen kode yang sama yang diperoleh dengan menyalin dan menempel satu atau dua variabel, tetapi pada saat yang sama beberapa variabel “diremehkan”.

Kode di atas menunjukkan ini. Di blok pertama, tindakan dilakukan dengan variabel localArchives , di blok berikutnya dari tipe yang sama, dengan localFiles . Dan jika Anda dengan hati-hati mempelajari kode ini, dan jangan membahasnya dengan cepat, seperti yang sering terjadi dengan ulasan kode, maka perhatikan tempat di mana Anda lupa mengganti variabel localArchives .

Pengawasan seperti itu dapat mengarah pada skenario berikut:

  • Misalkan kita memiliki localArchives (size = 4) dan localFiles (size = 2);
  • Saat membuat array localFiles.toArray (String baru [localArchives.size ()])) , kita mendapatkan 2 elemen terakhir menjadi null (["pathToFile1", "pathToFile2", null, null]);
  • Setelah itu, org.apache.hadoop.util.StringUtils.arrayToString akan mengembalikan representasi string array kami di mana nama file terakhir akan direpresentasikan sebagai "null" ("pathToFile1, pathToFile2, null, null" ) ;
  • Semua ini akan diteruskan, dan siapa yang tahu pemeriksaan apa yang ada untuk kasus-kasus seperti itu =).

Ekspresi V6007 'children.size ()> 0' selalu benar. Queue.java (347)

 boolean isHierarchySameAs(Queue newState) { .... if (children == null || children.size() == 0) { .... } else if(children.size() > 0) { .... } .... }

Karena fakta bahwa memeriksa jumlah elemen pada 0 dilakukan secara terpisah, pemeriksaan lebih lanjut children.size ()> 0 akan selalu memberikan true.

HDFS


V6001 Ada sub-ekspresi identik 'this.bucketSize' di sebelah kiri dan di sebelah kanan operator '%'. RollingWindow.java (79)

  RollingWindow(int windowLenMs, int numBuckets) { buckets = new Bucket[numBuckets]; for (int i = 0; i < numBuckets; i++) { buckets[i] = new Bucket(); } this.windowLenMs = windowLenMs; this.bucketSize = windowLenMs / numBuckets; if (this.bucketSize % bucketSize != 0) { // <= throw new IllegalArgumentException( "The bucket size in the rolling window is not integer: windowLenMs= " + windowLenMs + " numBuckets= " + numBuckets); } }

Cacat ini terletak pada fakta bahwa variabel dibagi menjadi dirinya sendiri. Akibatnya, pemeriksaan untuk multiplisitas akan selalu berlalu, dan, jika ada data input yang salah ( windowLenMs , numBuckets ), pengecualian tidak akan dibuang.

Benang


V6067 Dua atau lebih cabang kasus melakukan tindakan yang sama. TimelineEntityV2Converter.java (386), TimelineEntityV2Converter.java (389)

  public static ApplicationReport convertToApplicationReport(TimelineEntity entity) { .... if (metrics != null) { long vcoreSeconds = 0; long memorySeconds = 0; long preemptedVcoreSeconds = 0; long preemptedMemorySeconds = 0; for (TimelineMetric metric : metrics) { switch (metric.getId()) { case ApplicationMetricsConstants.APP_CPU_METRICS: vcoreSeconds = getAverageValue(metric.getValues().values()); break; case ApplicationMetricsConstants.APP_MEM_METRICS: memorySeconds = ....; break; case ApplicationMetricsConstants.APP_MEM_PREEMPT_METRICS: preemptedVcoreSeconds = ....; // <= break; case ApplicationMetricsConstants.APP_CPU_PREEMPT_METRICS: preemptedVcoreSeconds = ....; // <= break; default: // Should not happen.. break; } } .... } .... }

Dalam dua cabang kasus , fragmen kode yang sama. Ini terjadi setiap saat! Dalam jumlah kasus yang dominan, ini bukan kesalahan nyata, tetapi hanya kesempatan untuk berpikir tentang beralih refactoring. Tetapi tidak untuk kasus yang dimaksud. Dalam cuplikan kode berulang, nilai variabel preemptedVcoreSeconds ditetapkan . Jika Anda memperhatikan nama semua variabel dan konstanta, Anda bisa sampai pada kesimpulan bahwa dalam kasus metric.getId () == APP_MEM_PREEMPT_METRICS , nilai variabel preemptedMemorySeconds harus ditetapkan, bukan preemptedVcoreSeconds . Dalam hal ini, preemptedMemorySeconds akan selalu tetap 0 setelah mengeksekusi pernyataan 'switch', dan nilai preemptedVcoreSeconds mungkin salah.

V6046 Format salah. Jumlah item format yang berbeda diharapkan. Argumen tidak digunakan: 2. AbstractSchedulerPlanFollower.java (186)

 @Override public synchronized void synchronizePlan(Plan plan, boolean shouldReplan) { .... try { setQueueEntitlement(planQueueName, ....); } catch (YarnException e) { LOG.warn("Exception while trying to size reservation for plan: {}", currResId, planQueueName, e); } .... }

PlanQueueName variabel yang tidak digunakan saat login. Di sini, mereka terlalu banyak menyalin, atau tidak mengubah string format. Namun demikian, saya cenderung kepada orang tua yang baik dan, kadang-kadang membawa kerugian, salin-tempel.

Kode uji


Hadoop biasa


V6072 Dua fragmen kode serupa ditemukan. Mungkin, ini adalah kesalahan ketik dan variabel 'allSecretsB' harus digunakan alih-alih 'allSecretsA'. TestZKSignerSecretProvider.java (316), TestZKSignerSecretProvider.java (309), TestZKSignerSecretProvider.java (306), TestZKSignerSecretProvider.java (313)

 public void testMultiple(int order) throws Exception { .... currentSecretA = secretProviderA.getCurrentSecret(); allSecretsA = secretProviderA.getAllSecrets(); Assert.assertArrayEquals(secretA2, currentSecretA); Assert.assertEquals(2, allSecretsA.length); // <= Assert.assertArrayEquals(secretA2, allSecretsA[0]); Assert.assertArrayEquals(secretA1, allSecretsA[1]); currentSecretB = secretProviderB.getCurrentSecret(); allSecretsB = secretProviderB.getAllSecrets(); Assert.assertArrayEquals(secretA2, currentSecretB); Assert.assertEquals(2, allSecretsA.length); // <= Assert.assertArrayEquals(secretA2, allSecretsB[0]); Assert.assertArrayEquals(secretA1, allSecretsB[1]); .... }

Dan lagi V6072. Watch out for the variabel allSecretsA dan allSecretsB .

V6043 Pertimbangkan untuk memeriksa operator 'untuk'. Nilai awal dan akhir dari iterator adalah sama. TestTFile.java (235)

 private int readPrepWithUnknownLength(Scanner scanner, int start, int n) throws IOException { for (int i = start; i < start; i++) { String key = String.format(localFormatter, i); byte[] read = readKey(scanner); assertTrue("keys not equal", Arrays.equals(key.getBytes(), read)); try { read = readValue(scanner); assertTrue(false); } catch (IOException ie) { // should have thrown exception } String value = "value" + key; read = readLongValue(scanner, value.getBytes().length); assertTrue("values nto equal", Arrays.equals(read, value.getBytes())); scanner.advance(); } return (start + n); }

Tes yang selalu hijau? =). Tubuh loop, yang merupakan bagian dari tes, tidak pernah dieksekusi. Ini disebabkan oleh fakta bahwa nilai awal dan akhir dari pencocokan pencocokan dalam pernyataan for . Akibatnya, kondisi yang saya mulai akan segera memberi kita salah, yang akan mengarah pada perilaku ini. Saya berlari melalui file dengan tes dan sampai pada kesimpulan bahwa itu diperlukan untuk menulis dalam kondisi loop i <(start + n) .

Kurangi peta


a href = " www.viva64.com/en/w/v6007 "> Ekspresi V6007 'byteAm <0' selalu salah. DataWriter.java (322)

 GenerateOutput writeSegment(long byteAm, OutputStream out) throws IOException { long headerLen = getHeaderLength(); if (byteAm < headerLen) { // not enough bytes to write even the header return new GenerateOutput(0, 0); } // adjust for header length byteAm -= headerLen; if (byteAm < 0) { // <= byteAm = 0; } .... }

Kondisi byteAm <0 selalu salah. Untuk memahami, mari kita naikkan kode di atas. Jika eksekusi pengujian mencapai operasi byteAm - = headerLen , maka ini berarti bahwa akan ada byteAm> = headerLen . Dari sini, setelah melakukan pengurangan, nilai byteAm tidak akan pernah negatif. Yang harus dibuktikan.

HDFS


V6072 Dua fragmen kode serupa ditemukan. Mungkin, ini adalah kesalahan ketik dan variabel 'normalFile' harus digunakan daripada 'normalDir'. TestWebHDFS.java (625), TestWebHDFS.java (615), TestWebHDFS.java (614), TestWebHDFS.java (624)

 public void testWebHdfsErasureCodingFiles() throws Exception { .... final Path normalDir = new Path("/dir"); dfs.mkdirs(normalDir); final Path normalFile = new Path(normalDir, "file.log"); .... // logic block #1 FileStatus expectedNormalDirStatus = dfs.getFileStatus(normalDir); FileStatus actualNormalDirStatus = webHdfs.getFileStatus(normalDir); // <= Assert.assertEquals(expectedNormalDirStatus.isErasureCoded(), actualNormalDirStatus.isErasureCoded()); ContractTestUtils.assertNotErasureCoded(dfs, normalDir); assertTrue(normalDir + " should have erasure coding unset in " + ....); // logic block #2 FileStatus expectedNormalFileStatus = dfs.getFileStatus(normalFile); FileStatus actualNormalFileStatus = webHdfs.getFileStatus(normalDir); // <= Assert.assertEquals(expectedNormalFileStatus.isErasureCoded(), actualNormalFileStatus.isErasureCoded()); ContractTestUtils.assertNotErasureCoded(dfs, normalFile); assertTrue( normalFile + " should have erasure coding unset in " + ....); }

Jangan percaya, dan lagi V6072! Cukup ikuti variabel normalDir dan normalFile

V6027 Variabel diinisialisasi melalui panggilan ke fungsi yang sama. Mungkin salah atau kode tidak dioptimalkan. TestDFSAdmin.java (883), TestDFSAdmin.java (879)

 private void verifyNodesAndCorruptBlocks( final int numDn, final int numLiveDn, final int numCorruptBlocks, final int numCorruptECBlockGroups, final DFSClient client, final Long highestPriorityLowRedundancyReplicatedBlocks, final Long highestPriorityLowRedundancyECBlocks) throws IOException { /* init vars */ .... final String expectedCorruptedECBlockGroupsStr = String.format( "Block groups with corrupt internal blocks: %d", numCorruptECBlockGroups); final String highestPriorityLowRedundancyReplicatedBlocksStr = String.format( "\tLow redundancy blocks with highest priority " + "to recover: %d", highestPriorityLowRedundancyReplicatedBlocks); final String highestPriorityLowRedundancyECBlocksStr = String.format( "\tLow redundancy blocks with highest priority " + "to recover: %d", highestPriorityLowRedundancyReplicatedBlocks); .... }

Dalam fragmen ini, variabel tertinggiPriorityLowRedundancyReplicatedBlocksStr dan tertinggiPriorityLowRedundancyECBlocksStr diinisialisasi dengan nilai yang sama. Seringkali seharusnya demikian, tetapi tidak dalam situasi ini. Nama-nama variabel di sini panjang dan mirip satu sama lain, jadi saya tidak terkejut bahwa tidak ada modifikasi yang sesuai dengan copy-paste. Untuk memperbaiki situasi, saat menginisialisasi variabel tertinggiPriorityLowRedundancyECBlocksStr, Anda harus menggunakan parameter input tertinggiPriorityLowRedundancyECBlocks . Selain itu, kemungkinan besar, Anda masih perlu memperbaiki string format.

V6019 Kode tidak terjangkau terdeteksi. Mungkin saja ada kesalahan. TestReplaceDatanodeFailureReplication.java (222)

 private void verifyFileContent(...., SlowWriter[] slowwriters) throws IOException { LOG.info("Verify the file"); for (int i = 0; i < slowwriters.length; i++) { LOG.info(slowwriters[i].filepath + ....); FSDataInputStream in = null; try { in = fs.open(slowwriters[i].filepath); for (int j = 0, x;; j++) { x = in.read(); if ((x) != -1) { Assert.assertEquals(j, x); } else { return; } } } finally { IOUtils.closeStream(in); } } }

Analisator bersumpah bahwa mengubah penghitung i ++ di loop tidak dapat dicapai. Ini berarti bahwa dalam loop for (int i = 0; i <slowwriters.length; i ++) {....} tidak lebih dari satu iterasi yang akan dilakukan. Mari kita cari tahu alasannya. Jadi, dalam iterasi pertama, kami mengaitkan aliran dengan file yang sesuai dengan slowwriters [0] untuk dibaca lebih lanjut. Melalui for loop (int j = 0, x ;; j ++), kita membaca isi file dengan byte, di mana:

  • jika kita membaca sesuatu yang memadai, maka melalui assertEquals kita membandingkan byte baca dengan nilai saat ini dari counter j (dalam kasus verifikasi yang gagal, kita keluar dari tes dengan gagal)
  • jika file lulus tes dan kami mencapai akhir file (baca -1), maka kami keluar dari metode.

Oleh karena itu, apa pun yang terjadi ketika memeriksa slowwriter [0] , itu tidak akan sampai pada verifikasi elemen-elemen berikut. Kemungkinan besar, break harus digunakan alih-alih kembali .

Benang


V6019 Kode tidak terjangkau terdeteksi. Mungkin saja ada kesalahan. TestNodeManager.java (176)

 @Test public void testCreationOfNodeLabelsProviderService() throws InterruptedException { try { .... } catch (Exception e) { Assert.fail("Exception caught"); e.printStackTrace(); } }

Dalam situasi ini, stacktrace tidak akan pernah dicetak jika pengecualian terjadi, karena metode Assert.fail akan mengganggu pengujian. Jika ada cukup banyak pesan bahwa pengecualian ditangkap, maka agar tidak bingung, cetakan stacktrace perlu dihapus. Jika perlu mencetak, maka Anda hanya perlu menukarnya.

Ada banyak tempat seperti itu:
  • V6019 Kode tidak terjangkau terdeteksi. Mungkin saja ada kesalahan. TestResourceTrackerService.java (928)
  • V6019 Kode tidak terjangkau terdeteksi. Mungkin saja ada kesalahan. TestResourceTrackerService.java (737)
  • V6019 Kode tidak terjangkau terdeteksi. Mungkin saja ada kesalahan. TestResourceTrackerService.java (685)
  • ....

V6072 Dua fragmen kode serupa ditemukan. Mungkin, ini adalah kesalahan ketik dan variabel 'publicCache' harus digunakan daripada 'usercache'. TestResourceLocalizationService.java (315), TestResourceLocalizationService.java (309), TestResourceLocalizationService.java (307), TestResourceLocalizationService.java (313)

 @Test public void testDirectoryCleanupOnNewlyCreatedStateStore() throws IOException, URISyntaxException { .... // verify directory creation for (Path p : localDirs) { p = new Path((new URI(p.toString())).getPath()); // logic block #1 Path usercache = new Path(p, ContainerLocalizer.USERCACHE); verify(spylfs).rename(eq(usercache), any(Path.class), any()); // <= verify(spylfs).mkdir(eq(usercache), ....); // logic block #2 Path publicCache = new Path(p, ContainerLocalizer.FILECACHE); verify(spylfs).rename(eq(usercache), any(Path.class), any()); // <= verify(spylfs).mkdir(eq(publicCache), ....); .... } .... }

Dan akhirnya, sekali lagi V6072 =). Variabel untuk membiasakan diri Anda dengan cuplikan yang mencurigakan: usercache dan publicCache .

Kesimpulan


Selama pengembangan, ratusan ribu baris kode ditulis. Jika kode produksi berusaha menjaga kebersihan dari bug, cacat, dan kekurangan (pengembang menguji kode sendiri, melakukan tinjauan kode, dan banyak lagi), maka pengujian jelas lebih rendah dari ini. Cacat dalam tes diam-diam bisa bersembunyi di balik "centang hijau". Dan seperti yang Anda pahami dari analisis peringatan hari ini, tes yang berhasil lulus masih jauh dari tes yang dijamin.

Ketika memeriksa basis kode Apache Hadoop, analisis statis menunjukkan kebutuhannya tidak hanya untuk kode yang masuk ke produksi, tetapi juga untuk tes yang juga memainkan peran penting dalam pengembangan.

Jadi jika Anda peduli dengan kualitas kode dan basis pengujian Anda, maka saya sarankan Anda melihat analisis statis. Dan pelamar pertama untuk tes saya mengusulkan untuk mencoba PVS-Studio .



Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: Maxim Stefanov. Kualitas Kode Apache Hadoop: Produksi VS Tes .

Source: https://habr.com/ru/post/id480928/

More articles:


All Articles