HADOOP-17065. Add Network Counters to ABFS #2056

mehakmeet · 2020-06-05T15:44:27Z

Contributed by: Mehakmeet Singh

Tested by: mvn -T 1C -Dparallel-tests=abfs clean verify
Region: East US, West US

[INFO] Results:
[INFO]
[INFO] Tests run: 77, Failures: 0, Errors: 0, Skipped: 0

[INFO] Results:
[INFO]
[WARNING] Tests run: 442, Failures: 0, Errors: 0, Skipped: 70

[INFO] Results:
[INFO]
[WARNING] Tests run: 206, Failures: 0, Errors: 0, Skipped: 29

steveloughran

overall patch looks great!

some minor tuning of statistic names/descriptions and the tests, and it will be good to go

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsStatistic.java

steveloughran · 2020-06-12T09:46:54Z

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsStatistic.java

+  GET_RESPONSE("get_response",
+      "Total number of times response was recorded after sending requests."),
+  BYTES_SEND("bytes_send",
+      "Total bytes sent through http requests."),


how about "bytes sent from Azure Datalake"

wouldn't it be "bytes sent to Azure Datalake"?

yes. Or just "bytes uploaded". avoids having to deal with any future rebranding

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsStatistic.java

...tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java

...ols/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsNetworkStatistics.java

hadoop-yetus · 2020-06-14T04:07:46Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 30s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s	The patch appears to include 4 new or modified test files.
		_ trunk Compile Tests _
+1 💚	mvninstall	19m 4s	trunk passed
+1 💚	compile	0m 32s	trunk passed
+1 💚	checkstyle	0m 25s	trunk passed
+1 💚	mvnsite	0m 35s	trunk passed
+1 💚	shadedclient	14m 37s	branch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 27s	trunk passed
+0 🆗	spotbugs	0m 52s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	0m 49s	trunk passed
		_ Patch Compile Tests _
+1 💚	mvninstall	0m 28s	the patch passed
+1 💚	compile	0m 23s	the patch passed
+1 💚	javac	0m 23s	the patch passed
+1 💚	checkstyle	0m 16s	the patch passed
+1 💚	mvnsite	0m 27s	the patch passed
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	shadedclient	13m 38s	patch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 23s	the patch passed
+1 💚	findbugs	0m 55s	the patch passed
		_ Other Tests _
+1 💚	unit	1m 17s	hadoop-azure in the patch passed.
+1 💚	asflicense	0m 31s	The patch does not generate ASF License warnings.
		56m 52s

Subsystem	Report/Notes
Docker	ClientAPI=1.40 ServerAPI=1.40 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-2056/2/artifact/out/Dockerfile
GITHUB PR	#2056
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux 57c0ff64612f 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/hadoop.sh
git revision	trunk / `719b53a`
Default Java	Private Build-1.8.0_252-8u252-b09-1~18.04-b09
Test Results	https://builds.apache.org/job/hadoop-multibranch/job/PR-2056/2/testReport/
Max. process+thread count	420 (vs. ulimit of 5500)
modules	C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure
Console output	https://builds.apache.org/job/hadoop-multibranch/job/PR-2056/2/console
versions	git=2.17.1 maven=3.6.0 findbugs=3.1.0-RC1
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

...ols/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsNetworkStatistics.java

mukund-thakur · 2020-06-15T08:46:58Z

...tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java

    } else {
      this.client = new AbfsClient(baseUrl, creds, abfsConfiguration,
          new ExponentialRetryPolicy(abfsConfiguration.getMaxIoRetries()),
-          sasTokenProvider, abfsPerfTracker);
+          sasTokenProvider, abfsPerfTracker, instrumentation);


At this point the variable name is instrumentation and then it AbfsClient() it is statistics. I know these classes has been created earlier but I feel the names are bit confusing.
A few suggestions.

AbfsCounters could be AbfsInstrumentation and AbfsInstrumentation could be AbfsInstrumentationImpl

Let AbfsCounters be same. change AbfsInstrumentation to AbfsCountersImpl.
It is not important to change but if everybody feels the same then sooner the better.
CC @steveloughran

mukund-thakur · 2020-06-15T08:48:34Z

LGTM. Great patch @mehakmeet

hadoop-yetus · 2020-06-16T14:11:32Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	24m 5s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 1s	No case conflicting files found.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s	The patch appears to include 6 new or modified test files.
		_ trunk Compile Tests _
+1 💚	mvninstall	19m 9s	trunk passed
+1 💚	compile	0m 31s	trunk passed
+1 💚	checkstyle	0m 25s	trunk passed
+1 💚	mvnsite	0m 33s	trunk passed
+1 💚	shadedclient	14m 54s	branch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 27s	trunk passed
+0 🆗	spotbugs	0m 52s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	0m 49s	trunk passed
-0 ⚠️	patch	1m 8s	Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary.
		_ Patch Compile Tests _
+1 💚	mvninstall	0m 29s	the patch passed
+1 💚	compile	0m 24s	the patch passed
+1 💚	javac	0m 24s	the patch passed
+1 💚	checkstyle	0m 16s	the patch passed
+1 💚	mvnsite	0m 27s	the patch passed
+1 💚	whitespace	0m 1s	The patch has no whitespace issues.
+1 💚	shadedclient	13m 40s	patch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 23s	the patch passed
+1 💚	findbugs	0m 55s	the patch passed
		_ Other Tests _
+1 💚	unit	1m 17s	hadoop-azure in the patch passed.
+1 💚	asflicense	0m 30s	The patch does not generate ASF License warnings.
		80m 38s

Subsystem	Report/Notes
Docker	ClientAPI=1.40 ServerAPI=1.40 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-2056/3/artifact/out/Dockerfile
GITHUB PR	#2056
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux f77fc77c598a 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/hadoop.sh
git revision	trunk / `81d8a88`
Default Java	Private Build-1.8.0_252-8u252-b09-1~18.04-b09
Test Results	https://builds.apache.org/job/hadoop-multibranch/job/PR-2056/3/testReport/
Max. process+thread count	414 (vs. ulimit of 5500)
modules	C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure
Console output	https://builds.apache.org/job/hadoop-multibranch/job/PR-2056/3/console
versions	git=2.17.1 maven=3.6.0 findbugs=3.1.0-RC1
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

steveloughran

lgtm. we just need to agree on the name of the bytes sent counter

mehakmeet · 2020-06-17T14:02:49Z

I have added "Azure Datalake" to most of these network counters. I agree with the future rebranding issue, and I think I should make them more generalized.

hadoop-yetus · 2020-06-18T12:23:19Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 31s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 1s	No case conflicting files found.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s	The patch appears to include 6 new or modified test files.
		_ trunk Compile Tests _
+1 💚	mvninstall	18m 45s	trunk passed
+1 💚	compile	0m 32s	trunk passed
+1 💚	checkstyle	0m 26s	trunk passed
+1 💚	mvnsite	0m 35s	trunk passed
+1 💚	shadedclient	14m 43s	branch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 29s	trunk passed
+0 🆗	spotbugs	0m 52s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	0m 51s	trunk passed
-0 ⚠️	patch	1m 10s	Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary.
		_ Patch Compile Tests _
+1 💚	mvninstall	0m 28s	the patch passed
+1 💚	compile	0m 24s	the patch passed
+1 💚	javac	0m 24s	the patch passed
+1 💚	checkstyle	0m 17s	the patch passed
+1 💚	mvnsite	0m 28s	the patch passed
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	shadedclient	13m 48s	patch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 23s	the patch passed
+1 💚	findbugs	0m 54s	the patch passed
		_ Other Tests _
+1 💚	unit	1m 25s	hadoop-azure in the patch passed.
+1 💚	asflicense	0m 31s	The patch does not generate ASF License warnings.
		57m 3s

Subsystem	Report/Notes
Docker	ClientAPI=1.40 ServerAPI=1.40 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-2056/4/artifact/out/Dockerfile
GITHUB PR	#2056
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux 76b3e85405d6 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/hadoop.sh
git revision	trunk / `9cbd76c`
Default Java	Private Build-1.8.0_252-8u252-b09-1~18.04-b09
Test Results	https://builds.apache.org/job/hadoop-multibranch/job/PR-2056/4/testReport/
Max. process+thread count	415 (vs. ulimit of 5500)
modules	C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure
Console output	https://builds.apache.org/job/hadoop-multibranch/job/PR-2056/4/console
versions	git=2.17.1 maven=3.6.0 findbugs=3.1.0-RC1
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

mehakmeet · 2020-06-18T15:05:05Z

@steveloughran I am not able to understand the -0 patch error. I made a new branch locally and checked if there were any conflicts after rebasing and there weren't any.

cut an accidentally pasted test name and add "a" in some of the descriptions

Contributed by Mehakmeet Singh.

Contributed by Mehakmeet Singh. (cherry picked from commit 3472c3e)

Contributed by Mehakmeet Singh. Conflicts: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java Change-Id: I3fbfc5bbef00e39dbca4fbea69638d0e78b3cfb5

HADOOP-17065. Adding Network Counters in ABFS

2194104

steveloughran requested changes Jun 12, 2020

View reviewed changes

HADOOP-17065. fix review comments

223f9ad

mehakmeet requested a review from steveloughran June 14, 2020 02:52

mukund-thakur reviewed Jun 15, 2020

View reviewed changes

...ols/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsNetworkStatistics.java Outdated Show resolved Hide resolved

mukund-thakur reviewed Jun 15, 2020

View reviewed changes

HADOOP-17065. Renaming AbfsInstrumentation to AbfsCountersImpl

7b1d241

apache deleted a comment from hadoop-yetus Jun 17, 2020

steveloughran requested changes Jun 17, 2020

View reviewed changes

HADOOP-17065. AbfsStatistic descriptions.

7eb7e9d

steveloughran added 2 commits June 19, 2020 14:01

tweak some of the descriptions

c108dce

cut an accidentally pasted test name and add "a" in some of the descriptions

get my own edits right

bb3a4c9

steveloughran changed the title ~~HADOOP-17065. Adding Network Counters in ABFS~~ HADOOP-17065. Add Network Counters to ABFS Jun 19, 2020

steveloughran merged commit 3472c3e into apache:trunk Jun 19, 2020

mehakmeet mentioned this pull request Jul 3, 2020

Hadoop 16961. ABFS: Adding metrics to AbfsInputStream #2076

Merged

mehakmeet mentioned this pull request Jul 22, 2020

HADOOP-17113. Adding ReadAhead Counters in ABFS #2154

Merged

asfgit pushed a commit that referenced this pull request Jul 25, 2020

HADOOP-17065. Add Network Counters to ABFS (#2056)

bbd3278

Contributed by Mehakmeet Singh.

snvijaya pushed a commit to snvijaya/hadoop that referenced this pull request May 23, 2021

HADOOP-17065. Add Network Counters to ABFS (apache#2056)

da69a80

Contributed by Mehakmeet Singh.

arjun4084346 pushed a commit to arjun4084346/hadoop that referenced this pull request Apr 15, 2022

HADOOP-17065. Add Network Counters to ABFS (apache#2056)

61eb2cb

Contributed by Mehakmeet Singh. (cherry picked from commit 3472c3e)

arjun4084346 pushed a commit to arjun4084346/hadoop that referenced this pull request May 3, 2022

HADOOP-17065. Add Network Counters to ABFS (apache#2056)

0ca5415

Contributed by Mehakmeet Singh. (cherry picked from commit 3472c3e)

HADOOP-17065. Add Network Counters to ABFS #2056

HADOOP-17065. Add Network Counters to ABFS #2056

Uh oh!

Conversation

mehakmeet commented Jun 5, 2020

Uh oh!

steveloughran left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

steveloughran Jun 12, 2020

Choose a reason for hiding this comment

Uh oh!

mehakmeet Jun 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

steveloughran Jun 17, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hadoop-yetus commented Jun 14, 2020

Uh oh!

Uh oh!

mukund-thakur Jun 15, 2020

Choose a reason for hiding this comment

Uh oh!

mukund-thakur commented Jun 15, 2020

Uh oh!

hadoop-yetus commented Jun 16, 2020

Uh oh!

steveloughran left a comment

Choose a reason for hiding this comment

Uh oh!

mehakmeet commented Jun 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hadoop-yetus commented Jun 18, 2020

Uh oh!

mehakmeet commented Jun 18, 2020

Uh oh!

Uh oh!

mehakmeet Jun 14, 2020 •

edited

Loading

mehakmeet commented Jun 17, 2020 •

edited

Loading