Geekly articles weekly

اباتشي Hadoop كود الجودة: إنتاج اختبار VS

الشكل 1

من أجل الحصول على كود إنتاج عالي الجودة ، لا يكفي فقط توفير أقصى تغطية للاختبار. مما لا شك فيه ، من أجل تحقيق نتائج عالية ، يجب أن تعمل رمز المشروع الرئيسي والاختبارات في ترادف تام. لذلك ، تحتاج إلى إيلاء الاهتمام للاختبارات بقدر الكود الرئيسي. كتابة اختبار جيد هو المفتاح لالتقاط الانحدار في الإنتاج. لإظهار أهمية حقيقة أن الأخطاء في الاختبارات ليست أسوأ من الإنتاج ، سننظر في التحليل التالي للتحذيرات من محلل ثابت PVS-Studio. الهدف: اباتشي Hadoop.

عن المشروع


أولئك الذين كانوا مهتمين ذات يوم بالبيانات الكبيرة ربما سمعوا أو عملوا في مشروع مثل Apache Hadoop . باختصار ، Hadoop هو إطار يمكن استخدامه كأساس لبناء والعمل مع أنظمة البيانات الكبيرة.

يتكون Hadoop من أربع وحدات رئيسية ، تؤدي كل منها مهمة محددة ضرورية لنظام تحليل البيانات الضخم:

  • Hadoop المشتركة
  • مابريديوس
  • نظام الملفات الموزعة Hadoop (نظام الملفات الموزعة Hadoop)
  • غزل

ومع ذلك ، هناك الكثير من المواد للتعرف عليها على شبكة الإنترنت.

حول التحقق


كما هو موضح في الوثائق ، يمكن دمج PVS-Studio في المشروع بطرق مختلفة:

  • باستخدام البرنامج المساعد مخضرم.
  • باستخدام البرنامج المساعد المهد.
  • باستخدام IntellJ IDEA
  • باستخدام محلل مباشرة.

بنيت Hadoop على أساس نظام بناء مخضرم ، لذلك لم تكن هناك صعوبات مع التحقق.

بعد دمج البرنامج النصي من الوثائق وتعديله قليلاً من pom.xml (كانت هناك وحدات في التبعيات التي لم تكن موجودة) ، ذهب التحليل!

بعد التحليل ، باختيار التحذيرات الأكثر إثارة للاهتمام ، لاحظت أنني تلقيت نفس عدد التحذيرات في كل من كود الإنتاج والاختبارات. عادة ، أنا لا أعتبر محللًا يحفز الوقوع في الاختبارات. ولكن ، بتقسيمها ، لم أستطع التغيب عن التحذيرات من فئة "الاختبارات" بعد انتباهي. "لماذا لا؟" اعتقدت ، لأن الأخطاء في الاختبارات لها أيضًا عواقب. يمكن أن تؤدي إلى اختبار غير صحيح أو جزئي ، أو حتى إلى هراء (فقط للعرض ، بحيث تكون دائماً خضراء)

لذا ، وبعد جمع التحذيرات الأكثر إثارة للاهتمام ، وقسمها على الكود (الإنتاج ، الاختبار) ، ووحدات Hadoop الرئيسية الأربعة ، أوجه انتباهكم إلى تحليل لعمليات المحلل.

كود الانتاج


Hadoop المشتركة


V6033 تمت إضافة عنصر بنفس المفتاح "KDC_BIND_ADDRESS". 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); .... } .... }

القيمة المضافة مرتين إلى HashSet هي عيب شائع عند التحقق من المشاريع. في الواقع ، سيتم تجاهل الإضافة الثانية. حسنا ، إذا كان هذا الازدواجية حادث سخيف. ولكن ماذا لو كان يعني حقا إضافة قيمة أخرى؟

مابريديوس


V6072 تم العثور على شظايا كود مماثلة. ربما ، هذا خطأ مطبعي ويجب استخدام متغير "localFiles" بدلاً من "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()]))); } .... }

التشخيص V6072 يجعل في بعض الأحيان نتائج مثيرة للاهتمام للغاية. يتمثل جوهر التشخيص في البحث عن نفس النوع من أجزاء الشفرة التي تم الحصول عليها عن طريق نسخ ولصق واستبدال واحد أو اثنين من المتغيرات ، ولكن في الوقت نفسه "تم التقليل من شأن بعض المتغيرات".

الرمز أعلاه يوضح هذا. في الكتلة الأولى ، يتم تنفيذ الإجراءات باستخدام متغير localArchives ، في الكتلة التالية من نفس النوع ، مع ملف محلي . وإذا كنت تدرس هذا الرمز بضمير حي ، ولا تتخطاه بسرعة ، كما هو الحال غالبًا مع مراجعة التعليمات البرمجية ، فاحرص على ملاحظة المكان الذي نسيت فيه استبدال متغير LocalArchives .

يمكن أن تؤدي هذه الرقابة إلى السيناريو التالي:

  • افترض أن لدينا أرشيفية محلية (حجم = 4) وملف محلي (حجم = 2) ؛
  • عند إنشاء صفيف localFiles.toArray (سلسلة جديدة [localArchives.size ()]) ، نحصل على آخر عنصرين ليكونان فارغين (["pathToFile1" ، "pathToFile2" ، null ، null]) ؛
  • بعد ذلك ، سيعيد org.apache.hadoop.util.StringUtils.arrayToString تمثيل سلسلة من صفيفنا حيث سيتم تمثيل أسماء الملفات الأخيرة كـ "خالية" ("pathToFile1 ، pathToFile2 ، null ، null" ) ؛
  • سيتم تمرير كل هذا ، ومن يدري ما يتحقق هناك لمثل هذه الحالات =).

تعبير V6007 'children.size ()> 0' صحيح دائمًا. Queue.java (347)

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

نظرًا لأن التحقق من عدد العناصر عند 0 يتم بشكل منفصل ، فسيتم التحقق من صحة children.size ()> 0 دائمًا.

HDFS


V6001 هناك تعبيرات فرعية متطابقة 'this.bucketSize' إلى اليسار وإلى يمين عامل التشغيل '٪'. 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); } }

يكمن هذا العيب في حقيقة أن المتغير ينقسم إلى نفسه. نتيجة لذلك ، سيظل التحقق من التعددية دائمًا ، وفي حالة بيانات الإدخال غير الصحيحة ( windowLenMs ، numBuckets ) ، فلن يتم طرح الاستثناء.

غزل


V6067 يقوم اثنان أو أكثر من فروع الحالة بتنفيذ نفس الإجراءات. 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; } } .... } .... }

في اثنين من فروع القضية ، شظايا رمز نفسه. هذا يحدث في كل وقت! في العدد الغالب من الحالات ، ليس هذا خطأً حقيقياً ، بل هو مجرد فرصة للتفكير في إعادة تنظيم أماكن العمل. ولكن ليس للقضية المعنية. في تكرار مقتطفات الكود ، يتم تعيين قيمة المتغير preemptedVcoreSeconds . إذا كنت تهتم بأسماء جميع المتغيرات والثوابت ، يمكنك التوصل إلى استنتاج مفاده أنه في حالة metric.getId () == APP_MEM_PREEMPT_METRICS ، يجب تعيين قيمة المتغير preemptedMemorySeconds ، وليس preemptedVcoreSeconds . في هذا الصدد ، سيبقى برنامج preemptedMemorySeconds دائمًا 0 بعد تنفيذ عبارة 'switch' ، وقد تكون قيمة preemptedVcoreSeconds غير صحيحة.

V6046 تنسيق غير صحيح. من المتوقع وجود عدد مختلف من عناصر التنسيق. الوسيطات غير المستخدمة: 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 غير المستخدم عند التسجيل. هنا ، إما نسخهم أكثر من اللازم ، أو لم يعدلوا سلسلة التنسيق. لكن مع ذلك ، أميل إلى العجوز الجيد ، وأحيانًا ما أضر بالعيب.

رمز الاختبار


Hadoop المشتركة


V6072 تم العثور على شظايا كود مماثلة. ربما ، هذا خطأ مطبعي ويجب استخدام متغير "allSecretsB" بدلاً من "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]); .... }

ومرة أخرى V6072. احترس من المتغيرات allSecretsA و allSecretsB .

V6043 خذ بعين الاعتبار فحص المشغل 'for'. القيم الأولية والنهائية للتكرار هي نفسها. 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); }

اختبار أخضر دائمًا؟ =). جسد الحلقة ، التي تعد جزءًا من الاختبار ، لا يتم تنفيذها مطلقًا. هذا يرجع إلى حقيقة أن قيمتي البداية والنهاية لمطابقة العداد في العبارة for . ونتيجة لذلك ، فإن الشرط الذي سأبدأ به سيبدأ على الفور في تقديم معلومات خاطئة ، مما يؤدي إلى حدوث هذا السلوك. ركضت في الملف مع الاختبارات وتوصلت إلى استنتاج مفاده أنه كان مطلوبًا الكتابة في حالة الحلقة i <(start + n) .

مابريديوس


أ href = "www.viva64.com/en/w/v6007"> V6007 التعبير 'byteAm <0' غير صحيح دائمًا. 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; } .... }

شرط البايت <0 غير صحيح دائمًا. لفهم ، دعنا نرفع الكود أعلاه. إذا وصل تنفيذ الاختبار إلى العملية byteAm - = headerLen ، فهذا يعني أنه سيكون هناك byteAm> = headerLen . من هنا ، بعد إجراء الطرح ، لن تكون قيمة byteAm سالبًا. الذي كان مطلوبا لإثبات.

HDFS


V6072 تم العثور على شظايا كود مماثلة. ربما ، هذا خطأ مطبعي ويجب استخدام متغير "normalFile" بدلاً من "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 " + ....); }

لا أصدق ذلك ، ومرة ​​أخرى V6072! فقط اتبع متغيرات normalDir و normalFile

V6027 يتم تهيئة المتغيرات من خلال الدعوة إلى نفس الوظيفة. ربما يكون خطأ أو رمز un-optimized. 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); .... }

في هذه الشريحة ، تتم تهيئة المتغيرات highPriorityLowRedundancyReplicatedBlocksStr و topPriorityLowRedundancyECBlocksStr بنفس القيم. في كثير من الأحيان يجب أن يكون ، ولكن ليس في هذا الموقف. أسماء المتغيرات هنا طويلة وتشبه بعضها البعض ، لذلك لست مندهشًا من عدم وجود تعديلات مقابلة مع لصق النسخ. لتصحيح الموقف ، عند تهيئة المتغير higherPriorityLowRedundancyECBlocksStr ، تحتاج إلى استخدام معلمة الإدخال higherPriorityLowRedundancyECBlocks . بالإضافة إلى ذلك ، على الأرجح ، لا تزال بحاجة إلى إصلاح سلسلة التنسيق.

V6019 تم اكتشاف كود غير قابل للوصول. من الممكن وجود خطأ. 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); } } }

يقسم المحلل أن تغيير عداد i ++ في الحلقة لا يمكن تحقيقه. هذا يعني أنه في حلقة for (int i = 0 ؛ i <slowwriters.length؛ i ++) {....} لن يتم تنفيذ أكثر من تكرار واحد. دعنا نعرف لماذا. لذلك ، في التكرار الأول ، نربط الدفق بالملف المقابل للكتاب البطيئين [0] لمزيد من القراءة. من خلال حلقة for (int j = 0، x ؛؛ j ++) ، نقرأ محتويات الملف حسب البايتة ، حيث:

  • إذا قرأنا شيئًا مناسبًا ، فعندئذٍ من خلال assertEquals ، نقوم بمقارنة بايت القراءة بالقيمة الحالية لـ counter j (في حالة التحقق غير الناجح ، نخرج من الاختبار بالفشل) ؛
  • إذا اجتاز الملف الاختبار ووصلنا إلى نهاية الملف (اقرأ -1) ، فسنخرج من الطريقة.

لذلك ، بغض النظر عما يحدث عند التحقق من كتابات البطيئة [0] ، فلن يتحقق التحقق من العناصر التالية. على الأرجح ، يجب استخدام كسر بدلا من العودة .

غزل


V6019 تم اكتشاف كود غير قابل للوصول. من الممكن وجود خطأ. TestNodeManager.java (176)

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

في هذه الحالة ، لن تتم طباعة stacktrace أبدًا في حالة حدوث استثناء ، لأن طريقة Assert.fail ستقاطع الاختبار. إذا كانت هناك رسالة كافية تفيد بحدوث الاستثناء ، فمن أجل عدم الخلط ، يجب إزالة طباعة stacktrace'a. إذا كانت الطباعة ضرورية ، فأنت بحاجة فقط إلى تبديلها.

هناك العديد من هذه الأماكن:
  • 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)

 @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), ....); .... } .... }

وأخيرا ، مرة أخرى V6072 =). متغيرات للتعرف على المقتطف المشبوه: usercache و publicCache .

استنتاج


أثناء التطوير ، تتم كتابة مئات الآلاف من سطور التعليمات البرمجية. إذا كانت كود الإنتاج تحاول الحفاظ على نظافتها من الأخطاء والعيوب وأوجه القصور (يقوم المطور باختبار الكود الخاص به ، وإجراء مراجعة الكود ، وغير ذلك الكثير) ، فإن الاختبارات تكون أدنى من ذلك بوضوح. العيوب في الاختبارات يمكن أن تختبئ بهدوء وراء "علامة خضراء". وكما فهمت من تحليل اليوم للتحذيرات ، فإن الاختبار الناجح بنجاح ليس دائمًا اختبارًا مضمونًا.

عند التحقق من قاعدة بيانات Apache Hadoop ، أظهر التحليل الثابت حاجته ليس فقط للكود الذي يدخل في الإنتاج ، ولكن أيضًا للاختبارات التي تلعب أيضًا دورًا مهمًا في التطوير.

لذلك إذا كنت تهتم بجودة الشفرة وقاعدة الاختبار الخاصة بك ، فنوصيك بالاطلاع على التحليل الثابت. وأول مقدم طلب للاختبار أقترح تجربة PVS-Studio .



إذا كنت ترغب في مشاركة هذه المقالة مع جمهور يتحدث الإنجليزية ، فالرجاء استخدام الرابط الخاص بالترجمة: مكسيم ستيفانوف. اباتشي Hadoop رمز الجودة: الإنتاج مقابل الاختبار .

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

More articles:


All Articles