You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "virajjasani (via GitHub)" <gi...@apache.org> on 2023/03/20 19:31:45 UTC
[GitHub] [hadoop] virajjasani opened a new pull request, #5497: HDFS-16959. RBF: state store cache loading metrics
virajjasani opened a new pull request, #5497:
URL: https://github.com/apache/hadoop/pull/5497
With increasing num of state store records (like mount points), it would be good to be able to get the cache loading metrics like avg time for cache load during refresh, num of times cache is loaded etc.
<img width="428" alt="Screenshot 2023-03-19 at 11 58 24 PM" src="https://user-images.githubusercontent.com/34790606/226446060-54ac1f26-214a-4b2e-ba02-593ae12c39dc.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#issuecomment-1483941945
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 34s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +0 :ok: | markdownlint | 0m 0s | | markdownlint was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 16m 31s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 25m 55s | | trunk passed |
| +1 :green_heart: | compile | 23m 5s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | compile | 20m 30s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | checkstyle | 3m 47s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 43s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 12s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 2m 3s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 18s | | trunk passed |
| +1 :green_heart: | shadedclient | 21m 11s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 27s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 34s | | the patch passed |
| +1 :green_heart: | compile | 22m 26s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javac | 22m 26s | | the patch passed |
| +1 :green_heart: | compile | 20m 37s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | javac | 20m 37s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 3m 37s | | the patch passed |
| +1 :green_heart: | mvnsite | 2m 41s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 4s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 2m 4s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 23s | | the patch passed |
| +1 :green_heart: | shadedclient | 21m 31s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 22s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 20m 39s | | hadoop-hdfs-rbf in the patch passed. |
| +1 :green_heart: | asflicense | 1m 2s | | The patch does not generate ASF License warnings. |
| | | 247m 50s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/14/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5497 |
| Optional Tests | dupname asflicense mvnsite codespell detsecrets markdownlint compile javac javadoc mvninstall unit shadedclient spotbugs checkstyle |
| uname | Linux 429896a192cf 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / f85728166c608a84742fc383bce1d2370621a74a |
| Default Java | Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/14/testReport/ |
| Max. process+thread count | 2161 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-rbf U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/14/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] goiri merged pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "goiri (via GitHub)" <gi...@apache.org>.
goiri merged PR #5497:
URL: https://github.com/apache/hadoop/pull/5497
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#issuecomment-1482127778
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 56s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +0 :ok: | markdownlint | 0m 0s | | markdownlint was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 15m 30s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 28m 52s | | trunk passed |
| +1 :green_heart: | compile | 25m 6s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | compile | 21m 39s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | checkstyle | 3m 58s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 29s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 56s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 1m 44s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 10s | | trunk passed |
| +1 :green_heart: | shadedclient | 24m 5s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 23s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 34s | | the patch passed |
| +1 :green_heart: | compile | 24m 22s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javac | 24m 22s | | the patch passed |
| +1 :green_heart: | compile | 21m 43s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | javac | 21m 43s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 4m 18s | | the patch passed |
| +1 :green_heart: | mvnsite | 2m 30s | | the patch passed |
| +1 :green_heart: | javadoc | 1m 46s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 1m 46s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 22s | | the patch passed |
| +1 :green_heart: | shadedclient | 24m 30s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 15s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 21m 42s | | hadoop-hdfs-rbf in the patch passed. |
| +1 :green_heart: | asflicense | 0m 52s | | The patch does not generate ASF License warnings. |
| | | 261m 27s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/6/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5497 |
| Optional Tests | dupname asflicense mvnsite codespell detsecrets markdownlint compile javac javadoc mvninstall unit shadedclient spotbugs checkstyle |
| uname | Linux 244533ecfe8b 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / dd300b9bd9d758b0d3dd50ff8d95e7ca983bc154 |
| Default Java | Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/6/testReport/ |
| Max. process+thread count | 2429 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-rbf U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/6/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#issuecomment-1483735805
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 34s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets was not available. |
| +0 :ok: | markdownlint | 0m 1s | | markdownlint was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 16m 28s | | Maven dependency ordering for branch |
| -1 :x: | mvninstall | 25m 49s | [/branch-mvninstall-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/7/artifact/out/branch-mvninstall-root.txt) | root in trunk failed. |
| +1 :green_heart: | compile | 23m 3s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | compile | 20m 41s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | checkstyle | 3m 45s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 43s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 10s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 2m 1s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 17s | | trunk passed |
| -1 :x: | shadedclient | 21m 12s | | branch has errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 28s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 35s | | the patch passed |
| +1 :green_heart: | compile | 22m 23s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javac | 22m 23s | | the patch passed |
| +1 :green_heart: | compile | 20m 33s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | javac | 20m 33s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 3m 39s | | the patch passed |
| +1 :green_heart: | mvnsite | 2m 41s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 3s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 2m 8s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 27s | | the patch passed |
| -1 :x: | shadedclient | 21m 22s | | patch has errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 32s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 20m 43s | | hadoop-hdfs-rbf in the patch passed. |
| +1 :green_heart: | asflicense | 1m 2s | | The patch does not generate ASF License warnings. |
| | | 247m 56s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/7/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5497 |
| Optional Tests | dupname asflicense mvnsite codespell detsecrets markdownlint compile javac javadoc mvninstall unit shadedclient spotbugs checkstyle |
| uname | Linux 59257ff4b4f4 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 8847bbc3ebcfe9d82b517f6f26885f955c07effb |
| Default Java | Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/7/testReport/ |
| Max. process+thread count | 3147 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-rbf U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/7/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] MayankSinghParmar commented on pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "MayankSinghParmar (via GitHub)" <gi...@apache.org>.
MayankSinghParmar commented on PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#issuecomment-1568977449
I want to implement a new cache replacement algorithm in hdfs but I don't know which part of module to use and modify. Can any one please help me find the modules and functions which need to override to add my own caching replacement algorithm in hdfs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] virajjasani commented on a diff in pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#discussion_r1148404105
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreDriverBase.java:
##########
@@ -574,6 +580,38 @@ private static Map<String, Class<?>> getFields(BaseRecord record) {
return getters;
}
+ public long getMountTableCacheLoadSamples(StateStoreDriver driver) throws IOException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ return mountTableCache.lastStat().numSamples();
+ }
+
+ private static MutableRate getMountTableCache(StateStoreDriver driver) throws IOException {
+ StateStoreMetrics metrics = stateStore.getMetrics();
+ final Query<MountTable> query = new Query<>(MountTable.newInstance());
+ driver.getMultiple(MountTable.class, query);
+ final Map<String, MutableRate> cacheLoadMetrics = metrics.getCacheLoadMetrics();
+ final MutableRate mountTableCache = cacheLoadMetrics.get("CacheMountTableLoad");
+ assertNotNull("CacheMountTableLoad should be present in the state store metrics",
+ mountTableCache);
+ return mountTableCache;
+ }
+
+ public void testCacheLoadMetrics(StateStoreDriver driver, long numRefresh)
+ throws IOException, IllegalArgumentException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ // CacheMountTableLoadNumOps
+ final long mountTableCacheLoadNumOps = getMountTableCacheLoadSamples(driver);
+ assertEquals("Num of samples collected should match", numRefresh, mountTableCacheLoadNumOps);
+ // CacheMountTableLoadAvgTime ms
+ final double mountTableCacheLoadAvgTimeMs = mountTableCache.lastStat().mean();
+ // 2 seconds is a high enough value for the test, hence we expect mount table cache
+ // with very few entries to be loaded by this time duration, hence not have this test result
+ // show flaky behavior.
+ assertTrue(
+ "Mean time duration for cache load is expected to be less than 2000 ms. Actual value: "
+ + mountTableCacheLoadAvgTimeMs, mountTableCacheLoadAvgTimeMs < 2000d);
+ }
Review Comment:
Oh for prod, I think we should be better with ms itself because even with few entries I see at least some value in double for sure and that is always > 0 so I am sure we should be good there.
For test also, I just did more than 10 runs for each state store impl tests: fs, dfs, zk. And we definitely have values greater than 0. I think you are right that this should not be 0, let's go with 0 for now and if Jenkins QA complains, I would be happy to investigate further and we might apply comparison with -1 too. For now, 0 sounds good.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] ayushtkn commented on a diff in pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on code in PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#discussion_r1148399591
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreDriverBase.java:
##########
@@ -574,6 +580,38 @@ private static Map<String, Class<?>> getFields(BaseRecord record) {
return getters;
}
+ public long getMountTableCacheLoadSamples(StateStoreDriver driver) throws IOException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ return mountTableCache.lastStat().numSamples();
+ }
+
+ private static MutableRate getMountTableCache(StateStoreDriver driver) throws IOException {
+ StateStoreMetrics metrics = stateStore.getMetrics();
+ final Query<MountTable> query = new Query<>(MountTable.newInstance());
+ driver.getMultiple(MountTable.class, query);
+ final Map<String, MutableRate> cacheLoadMetrics = metrics.getCacheLoadMetrics();
+ final MutableRate mountTableCache = cacheLoadMetrics.get("CacheMountTableLoad");
+ assertNotNull("CacheMountTableLoad should be present in the state store metrics",
+ mountTableCache);
+ return mountTableCache;
+ }
+
+ public void testCacheLoadMetrics(StateStoreDriver driver, long numRefresh)
+ throws IOException, IllegalArgumentException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ // CacheMountTableLoadNumOps
+ final long mountTableCacheLoadNumOps = getMountTableCacheLoadSamples(driver);
+ assertEquals("Num of samples collected should match", numRefresh, mountTableCacheLoadNumOps);
+ // CacheMountTableLoadAvgTime ms
+ final double mountTableCacheLoadAvgTimeMs = mountTableCache.lastStat().mean();
+ // 2 seconds is a high enough value for the test, hence we expect mount table cache
+ // with very few entries to be loaded by this time duration, hence not have this test result
+ // show flaky behavior.
+ assertTrue(
+ "Mean time duration for cache load is expected to be less than 2000 ms. Actual value: "
+ + mountTableCacheLoadAvgTimeMs, mountTableCacheLoadAvgTimeMs < 2000d);
+ }
Review Comment:
Even if it goes to say 10K, it has nothing to do with your code, your code still captured the value and we would be happy with that.
Now if it is 0, it means it didn't capture the value, that is where we need to worried.
if there are chances it can stay at 0 :
- If only in test: May be have the default value as -1 and we can assert it isn't -1 and we should be sorted, if it is -1 then things didin't work the way we wanted.
- If it can be in prod as well: then we might have to think of lowering the unit or have dynamic units or so, because having these values as 0 in a prod cluster would be just confusing? mostly would create doubts like as if the metrics isn't getting captured.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] goiri commented on a diff in pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "goiri (via GitHub)" <gi...@apache.org>.
goiri commented on code in PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#discussion_r1148236769
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreDriverBase.java:
##########
@@ -574,6 +580,38 @@ private static Map<String, Class<?>> getFields(BaseRecord record) {
return getters;
}
+ public long getMountTableCacheLoadSamples(StateStoreDriver driver) throws IOException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ return mountTableCache.lastStat().numSamples();
+ }
+
+ private static MutableRate getMountTableCache(StateStoreDriver driver) throws IOException {
+ StateStoreMetrics metrics = stateStore.getMetrics();
+ final Query<MountTable> query = new Query<>(MountTable.newInstance());
+ driver.getMultiple(MountTable.class, query);
+ final Map<String, MutableRate> cacheLoadMetrics = metrics.getCacheLoadMetrics();
+ final MutableRate mountTableCache = cacheLoadMetrics.get("CacheMountTableLoad");
+ assertNotNull("CacheMountTableLoad should be present in the state store metrics",
+ mountTableCache);
+ return mountTableCache;
+ }
+
+ public void testCacheLoadMetrics(StateStoreDriver driver, long numRefresh)
+ throws IOException, IllegalArgumentException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ // CacheMountTableLoadNumOps
+ final long mountTableCacheLoadNumOps = getMountTableCacheLoadSamples(driver);
+ assertEquals("Num of samples collected should match", numRefresh, mountTableCacheLoadNumOps);
+ // CacheMountTableLoadAvgTime
+ final double mountTableCacheLoadAvgTime = mountTableCache.lastStat().mean();
Review Comment:
Add the unit "ms"
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreDriverBase.java:
##########
@@ -574,6 +580,38 @@ private static Map<String, Class<?>> getFields(BaseRecord record) {
return getters;
}
+ public long getMountTableCacheLoadSamples(StateStoreDriver driver) throws IOException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ return mountTableCache.lastStat().numSamples();
+ }
+
+ private static MutableRate getMountTableCache(StateStoreDriver driver) throws IOException {
+ StateStoreMetrics metrics = stateStore.getMetrics();
+ final Query<MountTable> query = new Query<>(MountTable.newInstance());
+ driver.getMultiple(MountTable.class, query);
+ final Map<String, MutableRate> cacheLoadMetrics = metrics.getCacheLoadMetrics();
+ final MutableRate mountTableCache = cacheLoadMetrics.get("CacheMountTableLoad");
+ assertNotNull("CacheMountTableLoad should be present in the state store metrics",
+ mountTableCache);
+ return mountTableCache;
+ }
+
+ public void testCacheLoadMetrics(StateStoreDriver driver, long numRefresh)
+ throws IOException, IllegalArgumentException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ // CacheMountTableLoadNumOps
+ final long mountTableCacheLoadNumOps = getMountTableCacheLoadSamples(driver);
+ assertEquals("Num of samples collected should match", numRefresh, mountTableCacheLoadNumOps);
+ // CacheMountTableLoadAvgTime
+ final double mountTableCacheLoadAvgTime = mountTableCache.lastStat().mean();
+ // 2000 ms is pretty high enough value for the test, hence we expect mount table cache
Review Comment:
```
// 2 seconds is a high enough value for the test, hence we expect mount table cache
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] virajjasani commented on a diff in pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#discussion_r1142815720
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreDriverBase.java:
##########
@@ -574,6 +578,15 @@ private static Map<String, Class<?>> getFields(BaseRecord record) {
return getters;
}
+ public void testCacheLoadMetrics(StateStoreDriver driver, int numRefresh)
+ throws IOException, IllegalArgumentException {
+ StateStoreMetrics metrics = stateStore.getMetrics();
+ final Query<MountTable> query = new Query<>(MountTable.newInstance());
+ driver.getMultiple(MountTable.class, query);
+ assertEquals(numRefresh,
Review Comment:
Done, added time as well. I have added time duration value high enough that it's highly unlikely that it could fail to assert the value.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#issuecomment-1477302012
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 55s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +0 :ok: | markdownlint | 0m 0s | | markdownlint was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 16m 8s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 27m 8s | | trunk passed |
| +1 :green_heart: | compile | 26m 17s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | compile | 23m 6s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | checkstyle | 3m 42s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 37s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 8s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 1m 55s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 20s | | trunk passed |
| +1 :green_heart: | shadedclient | 21m 8s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 29s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 34s | | the patch passed |
| +1 :green_heart: | compile | 25m 24s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javac | 25m 24s | | the patch passed |
| +1 :green_heart: | compile | 23m 11s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | javac | 23m 11s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 3m 33s | | the patch passed |
| +1 :green_heart: | mvnsite | 2m 32s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 5s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 2m 7s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 26s | | the patch passed |
| +1 :green_heart: | shadedclient | 20m 56s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 47s | | hadoop-common in the patch passed. |
| -1 :x: | unit | 21m 15s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/2/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) | hadoop-hdfs-rbf in the patch passed. |
| +1 :green_heart: | asflicense | 0m 58s | | The patch does not generate ASF License warnings. |
| | | 260m 18s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hdfs.server.federation.router.TestRouterRPCMultipleDestinationMountTableResolver |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5497 |
| Optional Tests | dupname asflicense mvnsite codespell detsecrets markdownlint compile javac javadoc mvninstall unit shadedclient spotbugs checkstyle |
| uname | Linux be08a72d3d64 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / c2d83283337a997559552ce5e4dde56c90945ee6 |
| Default Java | Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/2/testReport/ |
| Max. process+thread count | 2536 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-rbf U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/2/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] ayushtkn commented on a diff in pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on code in PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#discussion_r1146303204
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreDriverBase.java:
##########
@@ -574,6 +580,26 @@ private static Map<String, Class<?>> getFields(BaseRecord record) {
return getters;
}
+ public void testCacheLoadMetrics(StateStoreDriver driver, int numRefresh)
+ throws IOException, IllegalArgumentException {
+ StateStoreMetrics metrics = stateStore.getMetrics();
+ final Query<MountTable> query = new Query<>(MountTable.newInstance());
+ driver.getMultiple(MountTable.class, query);
+ final Map<String, MutableRate> cacheLoadMetrics = metrics.getCacheLoadMetrics();
+ final MutableRate mountTableCache = cacheLoadMetrics.get("CacheMountTableLoad");
+ assertNotNull("CacheMountTableLoad should be present in the state store metrics",
+ mountTableCache);
+ // CacheMountTableLoadNumOps
+ final long mountTableCacheLoadNumOps = mountTableCache.lastStat().numSamples();
+ assertEquals("Num of samples collected should match with " + numRefresh + ". Actual value: "
+ + mountTableCacheLoadNumOps, numRefresh, mountTableCacheLoadNumOps);
+ // CacheMountTableLoadAvgTime
+ final double mountTableCacheLoadAvgTime = mountTableCache.lastStat().mean();
+ assertTrue(
+ "Mean time duration for cache load is expected to be less than 2000 ms. Actual value: "
+ + mountTableCacheLoadAvgTime, mountTableCacheLoadAvgTime < 2000d);
Review Comment:
Can you drop a comment above explaining the reason why it should be ``less than 2000 ms``
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreDriverBase.java:
##########
@@ -574,6 +580,26 @@ private static Map<String, Class<?>> getFields(BaseRecord record) {
return getters;
}
+ public void testCacheLoadMetrics(StateStoreDriver driver, int numRefresh)
+ throws IOException, IllegalArgumentException {
+ StateStoreMetrics metrics = stateStore.getMetrics();
+ final Query<MountTable> query = new Query<>(MountTable.newInstance());
+ driver.getMultiple(MountTable.class, query);
+ final Map<String, MutableRate> cacheLoadMetrics = metrics.getCacheLoadMetrics();
+ final MutableRate mountTableCache = cacheLoadMetrics.get("CacheMountTableLoad");
+ assertNotNull("CacheMountTableLoad should be present in the state store metrics",
+ mountTableCache);
+ // CacheMountTableLoadNumOps
+ final long mountTableCacheLoadNumOps = mountTableCache.lastStat().numSamples();
+ assertEquals("Num of samples collected should match with " + numRefresh + ". Actual value: "
+ + mountTableCacheLoadNumOps, numRefresh, mountTableCacheLoadNumOps);
Review Comment:
Do we need to put ``numRefresh`` and ``mountTableCacheLoadNumOps`` in the message, ``assertEquals`` will print that right, if they aren't equal
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreZK.java:
##########
@@ -206,4 +206,13 @@ public void testFetchErrors()
stateStoreDriver.setEnableConcurrent(true);
testFetchErrors(stateStoreDriver);
}
+
+ @Test
+ public void testCacheLoadMetrics() throws IOException {
+ getStateStoreService().refreshCaches(true);
+ getStateStoreService().refreshCaches(true);
+ getStateStoreService().refreshCaches(true);
+ testCacheLoadMetrics(getStateStoreDriver(), 4);
Review Comment:
The hard coded number is dependent on other tests in the class, If you add a test which also does ``refreshCaches``, and then this test is executed this test will fail.
You need to keep these numbers isolated and safe from changing getting impacted by addition or change of nature of other tests.
A simple way to try is: Just copy this same test in the same class and run the entire class, whichever test run late will fail
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#issuecomment-1481924908
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 53s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +0 :ok: | markdownlint | 0m 0s | | markdownlint was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 15m 43s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 29m 8s | | trunk passed |
| +1 :green_heart: | compile | 25m 10s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | compile | 21m 44s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | checkstyle | 4m 1s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 29s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 51s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 1m 44s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 9s | | trunk passed |
| +1 :green_heart: | shadedclient | 24m 9s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 24s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 34s | | the patch passed |
| +1 :green_heart: | compile | 24m 29s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javac | 24m 29s | | the patch passed |
| +1 :green_heart: | compile | 21m 36s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | javac | 21m 36s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 3m 48s | | the patch passed |
| +1 :green_heart: | mvnsite | 2m 27s | | the patch passed |
| +1 :green_heart: | javadoc | 1m 46s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 1m 47s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 19s | | the patch passed |
| +1 :green_heart: | shadedclient | 24m 9s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 15s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 21m 55s | | hadoop-hdfs-rbf in the patch passed. |
| +1 :green_heart: | asflicense | 0m 53s | | The patch does not generate ASF License warnings. |
| | | 261m 22s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/5/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5497 |
| Optional Tests | dupname asflicense mvnsite codespell detsecrets markdownlint compile javac javadoc mvninstall unit shadedclient spotbugs checkstyle |
| uname | Linux 3fbf9f0033e7 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 1a503e2424ba39d1c51fc1a771342d51102cf377 |
| Default Java | Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/5/testReport/ |
| Max. process+thread count | 2416 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-rbf U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/5/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] virajjasani commented on a diff in pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#discussion_r1148397671
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreDriverBase.java:
##########
@@ -574,6 +580,38 @@ private static Map<String, Class<?>> getFields(BaseRecord record) {
return getters;
}
+ public long getMountTableCacheLoadSamples(StateStoreDriver driver) throws IOException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ return mountTableCache.lastStat().numSamples();
+ }
+
+ private static MutableRate getMountTableCache(StateStoreDriver driver) throws IOException {
+ StateStoreMetrics metrics = stateStore.getMetrics();
+ final Query<MountTable> query = new Query<>(MountTable.newInstance());
+ driver.getMultiple(MountTable.class, query);
+ final Map<String, MutableRate> cacheLoadMetrics = metrics.getCacheLoadMetrics();
+ final MutableRate mountTableCache = cacheLoadMetrics.get("CacheMountTableLoad");
+ assertNotNull("CacheMountTableLoad should be present in the state store metrics",
+ mountTableCache);
+ return mountTableCache;
+ }
+
+ public void testCacheLoadMetrics(StateStoreDriver driver, long numRefresh)
+ throws IOException, IllegalArgumentException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ // CacheMountTableLoadNumOps
+ final long mountTableCacheLoadNumOps = getMountTableCacheLoadSamples(driver);
+ assertEquals("Num of samples collected should match", numRefresh, mountTableCacheLoadNumOps);
+ // CacheMountTableLoadAvgTime ms
+ final double mountTableCacheLoadAvgTimeMs = mountTableCache.lastStat().mean();
+ // 2 seconds is a high enough value for the test, hence we expect mount table cache
+ // with very few entries to be loaded by this time duration, hence not have this test result
+ // show flaky behavior.
+ assertTrue(
+ "Mean time duration for cache load is expected to be less than 2000 ms. Actual value: "
+ + mountTableCacheLoadAvgTimeMs, mountTableCacheLoadAvgTimeMs < 2000d);
+ }
Review Comment:
Cool, that's fine. I am only worried about fast Jenkins env where just in case if the cache loading happens in nanoseconds, perhaps even milliseconds might be 0?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] virajjasani commented on a diff in pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#discussion_r1148404105
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreDriverBase.java:
##########
@@ -574,6 +580,38 @@ private static Map<String, Class<?>> getFields(BaseRecord record) {
return getters;
}
+ public long getMountTableCacheLoadSamples(StateStoreDriver driver) throws IOException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ return mountTableCache.lastStat().numSamples();
+ }
+
+ private static MutableRate getMountTableCache(StateStoreDriver driver) throws IOException {
+ StateStoreMetrics metrics = stateStore.getMetrics();
+ final Query<MountTable> query = new Query<>(MountTable.newInstance());
+ driver.getMultiple(MountTable.class, query);
+ final Map<String, MutableRate> cacheLoadMetrics = metrics.getCacheLoadMetrics();
+ final MutableRate mountTableCache = cacheLoadMetrics.get("CacheMountTableLoad");
+ assertNotNull("CacheMountTableLoad should be present in the state store metrics",
+ mountTableCache);
+ return mountTableCache;
+ }
+
+ public void testCacheLoadMetrics(StateStoreDriver driver, long numRefresh)
+ throws IOException, IllegalArgumentException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ // CacheMountTableLoadNumOps
+ final long mountTableCacheLoadNumOps = getMountTableCacheLoadSamples(driver);
+ assertEquals("Num of samples collected should match", numRefresh, mountTableCacheLoadNumOps);
+ // CacheMountTableLoadAvgTime ms
+ final double mountTableCacheLoadAvgTimeMs = mountTableCache.lastStat().mean();
+ // 2 seconds is a high enough value for the test, hence we expect mount table cache
+ // with very few entries to be loaded by this time duration, hence not have this test result
+ // show flaky behavior.
+ assertTrue(
+ "Mean time duration for cache load is expected to be less than 2000 ms. Actual value: "
+ + mountTableCacheLoadAvgTimeMs, mountTableCacheLoadAvgTimeMs < 2000d);
+ }
Review Comment:
Oh for prod, I think we should be better with ms itself because even with few entries I see at least some value in double for sure and that is always > 0 so I am sure we should be good there.
For test also, I just did more than 10 runs for each state store impl tests: fs, dfs, zk. And we definitely have values greater than 0 but one time I saw just 0.01d too hence I wonder Jenkins might get 0.0 too. Hence, let me go with your suggestion for test:
> If only in test: May be have the default value as -1 and we can assert it isn't -1 and we should be sorted, if it is -1 then things didin't work the way we wanted.
Let me make this change
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] virajjasani commented on a diff in pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#discussion_r1148404105
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreDriverBase.java:
##########
@@ -574,6 +580,38 @@ private static Map<String, Class<?>> getFields(BaseRecord record) {
return getters;
}
+ public long getMountTableCacheLoadSamples(StateStoreDriver driver) throws IOException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ return mountTableCache.lastStat().numSamples();
+ }
+
+ private static MutableRate getMountTableCache(StateStoreDriver driver) throws IOException {
+ StateStoreMetrics metrics = stateStore.getMetrics();
+ final Query<MountTable> query = new Query<>(MountTable.newInstance());
+ driver.getMultiple(MountTable.class, query);
+ final Map<String, MutableRate> cacheLoadMetrics = metrics.getCacheLoadMetrics();
+ final MutableRate mountTableCache = cacheLoadMetrics.get("CacheMountTableLoad");
+ assertNotNull("CacheMountTableLoad should be present in the state store metrics",
+ mountTableCache);
+ return mountTableCache;
+ }
+
+ public void testCacheLoadMetrics(StateStoreDriver driver, long numRefresh)
+ throws IOException, IllegalArgumentException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ // CacheMountTableLoadNumOps
+ final long mountTableCacheLoadNumOps = getMountTableCacheLoadSamples(driver);
+ assertEquals("Num of samples collected should match", numRefresh, mountTableCacheLoadNumOps);
+ // CacheMountTableLoadAvgTime ms
+ final double mountTableCacheLoadAvgTimeMs = mountTableCache.lastStat().mean();
+ // 2 seconds is a high enough value for the test, hence we expect mount table cache
+ // with very few entries to be loaded by this time duration, hence not have this test result
+ // show flaky behavior.
+ assertTrue(
+ "Mean time duration for cache load is expected to be less than 2000 ms. Actual value: "
+ + mountTableCacheLoadAvgTimeMs, mountTableCacheLoadAvgTimeMs < 2000d);
+ }
Review Comment:
Oh for prod, I think we should be better with ms itself because even with few entries I see at least some value in double for sure and that is always > 0 so I am sure we should be good there.
For test also, I just did more than 10 runs for each state store impl tests: fs, dfs, zk. And we definitely have values greater than 0 but one time I saw just 0.01d too hence I wonder Jenkins might get 0.0 too. Hence, let me go with your suggestion for test:
> If only in test: May be have the default value as -1 and we can assert it isn't -1 and we should be sorted, if it is -1 then things didin't work the way we wanted.
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] MayankSinghParmar commented on pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "MayankSinghParmar (via GitHub)" <gi...@apache.org>.
MayankSinghParmar commented on PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#issuecomment-1568984267
I want to implement a new cache replacement algorithm in hdfs but I don't know which part of module to use and modify. Can any one please help me find the modules and functions which need to override to add my own caching replacement algorithm in hdfs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] virajjasani commented on a diff in pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#discussion_r1146493478
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreDriverBase.java:
##########
@@ -574,6 +580,26 @@ private static Map<String, Class<?>> getFields(BaseRecord record) {
return getters;
}
+ public void testCacheLoadMetrics(StateStoreDriver driver, int numRefresh)
+ throws IOException, IllegalArgumentException {
+ StateStoreMetrics metrics = stateStore.getMetrics();
+ final Query<MountTable> query = new Query<>(MountTable.newInstance());
+ driver.getMultiple(MountTable.class, query);
+ final Map<String, MutableRate> cacheLoadMetrics = metrics.getCacheLoadMetrics();
+ final MutableRate mountTableCache = cacheLoadMetrics.get("CacheMountTableLoad");
+ assertNotNull("CacheMountTableLoad should be present in the state store metrics",
+ mountTableCache);
+ // CacheMountTableLoadNumOps
+ final long mountTableCacheLoadNumOps = mountTableCache.lastStat().numSamples();
+ assertEquals("Num of samples collected should match with " + numRefresh + ". Actual value: "
+ + mountTableCacheLoadNumOps, numRefresh, mountTableCacheLoadNumOps);
+ // CacheMountTableLoadAvgTime
+ final double mountTableCacheLoadAvgTime = mountTableCache.lastStat().mean();
+ assertTrue(
+ "Mean time duration for cache load is expected to be less than 2000 ms. Actual value: "
+ + mountTableCacheLoadAvgTime, mountTableCacheLoadAvgTime < 2000d);
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#issuecomment-1483944242
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 44s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +0 :ok: | markdownlint | 0m 0s | | markdownlint was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 15m 47s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 28m 52s | | trunk passed |
| +1 :green_heart: | compile | 25m 12s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | compile | 21m 48s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | checkstyle | 3m 59s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 30s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 54s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 1m 47s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 13s | | trunk passed |
| +1 :green_heart: | shadedclient | 24m 4s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 23s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 34s | | the patch passed |
| +1 :green_heart: | compile | 24m 35s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javac | 24m 35s | | the patch passed |
| +1 :green_heart: | compile | 21m 46s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | javac | 21m 46s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 3m 49s | | the patch passed |
| +1 :green_heart: | mvnsite | 2m 26s | | the patch passed |
| +1 :green_heart: | javadoc | 1m 46s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 1m 48s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 24s | | the patch passed |
| +1 :green_heart: | shadedclient | 24m 43s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 19s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 21m 41s | | hadoop-hdfs-rbf in the patch passed. |
| +1 :green_heart: | asflicense | 0m 52s | | The patch does not generate ASF License warnings. |
| | | 261m 45s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/15/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5497 |
| Optional Tests | dupname asflicense mvnsite codespell detsecrets markdownlint compile javac javadoc mvninstall unit shadedclient spotbugs checkstyle |
| uname | Linux 8f5b567aed03 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / f85728166c608a84742fc383bce1d2370621a74a |
| Default Java | Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/15/testReport/ |
| Max. process+thread count | 2425 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-rbf U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/15/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] goiri commented on a diff in pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "goiri (via GitHub)" <gi...@apache.org>.
goiri commented on code in PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#discussion_r1142801861
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/CachedRecordStore.java:
##########
@@ -141,8 +142,9 @@ public boolean loadCache(boolean force) throws IOException {
// Update the metrics for the cache State Store size
StateStoreMetrics metrics = getDriver().getMetrics();
+ String recordName = getRecordClass().getSimpleName();
Review Comment:
Shouldn't this be inside of the null check?
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreDriverBase.java:
##########
@@ -574,6 +578,15 @@ private static Map<String, Class<?>> getFields(BaseRecord record) {
return getters;
}
+ public void testCacheLoadMetrics(StateStoreDriver driver, int numRefresh)
+ throws IOException, IllegalArgumentException {
+ StateStoreMetrics metrics = stateStore.getMetrics();
+ final Query<MountTable> query = new Query<>(MountTable.newInstance());
+ driver.getMultiple(MountTable.class, query);
+ assertEquals(numRefresh,
+ metrics.getCacheLoadMetrics().get("CacheMountTableLoad").lastStat().numSamples());
Review Comment:
Extract this a little.
Probably `metrics.getCacheLoadMetrics()` and the final value.
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreDriverBase.java:
##########
@@ -574,6 +578,15 @@ private static Map<String, Class<?>> getFields(BaseRecord record) {
return getters;
}
+ public void testCacheLoadMetrics(StateStoreDriver driver, int numRefresh)
+ throws IOException, IllegalArgumentException {
+ StateStoreMetrics metrics = stateStore.getMetrics();
+ final Query<MountTable> query = new Query<>(MountTable.newInstance());
+ driver.getMultiple(MountTable.class, query);
+ assertEquals(numRefresh,
Review Comment:
Can we check for NumOps and Time?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] virajjasani commented on pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#issuecomment-1483149115
@goiri @ayushtkn Thanks a lot for your reviews! Does this look good to go?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] virajjasani commented on a diff in pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#discussion_r1142803647
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreDriverBase.java:
##########
@@ -574,6 +578,15 @@ private static Map<String, Class<?>> getFields(BaseRecord record) {
return getters;
}
+ public void testCacheLoadMetrics(StateStoreDriver driver, int numRefresh)
+ throws IOException, IllegalArgumentException {
+ StateStoreMetrics metrics = stateStore.getMetrics();
+ final Query<MountTable> query = new Query<>(MountTable.newInstance());
+ driver.getMultiple(MountTable.class, query);
+ assertEquals(numRefresh,
Review Comment:
The only problem with time duration check is that it could result in flaky tests in future. Is numOps alone fine?
NumOps is taken care of with `lastStat().numSamples()`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] virajjasani commented on a diff in pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#discussion_r1148404105
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreDriverBase.java:
##########
@@ -574,6 +580,38 @@ private static Map<String, Class<?>> getFields(BaseRecord record) {
return getters;
}
+ public long getMountTableCacheLoadSamples(StateStoreDriver driver) throws IOException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ return mountTableCache.lastStat().numSamples();
+ }
+
+ private static MutableRate getMountTableCache(StateStoreDriver driver) throws IOException {
+ StateStoreMetrics metrics = stateStore.getMetrics();
+ final Query<MountTable> query = new Query<>(MountTable.newInstance());
+ driver.getMultiple(MountTable.class, query);
+ final Map<String, MutableRate> cacheLoadMetrics = metrics.getCacheLoadMetrics();
+ final MutableRate mountTableCache = cacheLoadMetrics.get("CacheMountTableLoad");
+ assertNotNull("CacheMountTableLoad should be present in the state store metrics",
+ mountTableCache);
+ return mountTableCache;
+ }
+
+ public void testCacheLoadMetrics(StateStoreDriver driver, long numRefresh)
+ throws IOException, IllegalArgumentException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ // CacheMountTableLoadNumOps
+ final long mountTableCacheLoadNumOps = getMountTableCacheLoadSamples(driver);
+ assertEquals("Num of samples collected should match", numRefresh, mountTableCacheLoadNumOps);
+ // CacheMountTableLoadAvgTime ms
+ final double mountTableCacheLoadAvgTimeMs = mountTableCache.lastStat().mean();
+ // 2 seconds is a high enough value for the test, hence we expect mount table cache
+ // with very few entries to be loaded by this time duration, hence not have this test result
+ // show flaky behavior.
+ assertTrue(
+ "Mean time duration for cache load is expected to be less than 2000 ms. Actual value: "
+ + mountTableCacheLoadAvgTimeMs, mountTableCacheLoadAvgTimeMs < 2000d);
+ }
Review Comment:
Oh for prod, I think we should be better with ms itself because even with few entries I see at least some value in double for sure and that is always > 0 so I am sure we should be good there.
For test also, I just did more than 10 runs for each state store impl tests: fs, dfs, zk. And we definitely have values greater than 0 but one time I saw just 0.01d too hence I wonder Jenkins might get 0.0 too. Hence, let me go with your suggestion for test:
> If only in test: May be have the default value as -1 and we can assert it isn't -1 and we should be sorted, if it is -1 then things didin't work the way we wanted.
Let me make this change by comparing it to >= 0 so that we don't even have to worry about values b/ 0 and -1.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] ayushtkn commented on a diff in pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on code in PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#discussion_r1148534243
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreDriverBase.java:
##########
@@ -574,6 +580,36 @@ private static Map<String, Class<?>> getFields(BaseRecord record) {
return getters;
}
+ public long getMountTableCacheLoadSamples(StateStoreDriver driver) throws IOException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ return mountTableCache.lastStat().numSamples();
+ }
+
+ private static MutableRate getMountTableCache(StateStoreDriver driver) throws IOException {
+ StateStoreMetrics metrics = stateStore.getMetrics();
+ final Query<MountTable> query = new Query<>(MountTable.newInstance());
+ driver.getMultiple(MountTable.class, query);
+ final Map<String, MutableRate> cacheLoadMetrics = metrics.getCacheLoadMetrics();
+ final MutableRate mountTableCache = cacheLoadMetrics.get("CacheMountTableLoad");
+ assertNotNull("CacheMountTableLoad should be present in the state store metrics",
+ mountTableCache);
+ return mountTableCache;
+ }
+
+ public void testCacheLoadMetrics(StateStoreDriver driver, long numRefresh,
+ double expectedHigherThan) throws IOException, IllegalArgumentException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ // CacheMountTableLoadNumOps
+ final long mountTableCacheLoadNumOps = getMountTableCacheLoadSamples(driver);
+ assertEquals("Num of samples collected should match", numRefresh, mountTableCacheLoadNumOps);
+ // CacheMountTableLoadAvgTime ms
+ final double mountTableCacheLoadAvgTimeMs = mountTableCache.lastStat().mean();
+ assertTrue(
+ "Mean time duration for cache load is expected to be higher than " + expectedHigherThan
+ + " ms." + " Actual value: " + mountTableCacheLoadAvgTimeMs,
+ mountTableCacheLoadAvgTimeMs > expectedHigherThan);
+ }
Review Comment:
assertNotEquals could have also worked, but ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] virajjasani commented on pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#issuecomment-1483679610
Btw the build is broken after commit https://github.com/apache/hadoop/commit/72b01227062fcd5214c3561c4b81842ac1802bdd
just in case we get -1 for mvninstall from Jenkins QA
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] virajjasani commented on pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#issuecomment-1483673321
Done, thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#issuecomment-1483798975
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 35s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +0 :ok: | markdownlint | 0m 0s | | markdownlint was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 16m 34s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 26m 53s | | trunk passed |
| +1 :green_heart: | compile | 23m 23s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | compile | 20m 33s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | checkstyle | 3m 43s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 44s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 10s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 2m 7s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 16s | | trunk passed |
| +1 :green_heart: | shadedclient | 21m 12s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 28s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 34s | | the patch passed |
| +1 :green_heart: | compile | 22m 23s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javac | 22m 23s | | the patch passed |
| +1 :green_heart: | compile | 20m 37s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | javac | 20m 37s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 3m 41s | | the patch passed |
| +1 :green_heart: | mvnsite | 2m 41s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 3s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 2m 5s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 24s | | the patch passed |
| +1 :green_heart: | shadedclient | 21m 27s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 20s | | hadoop-common in the patch passed. |
| -1 :x: | unit | 20m 52s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/9/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) | hadoop-hdfs-rbf in the patch passed. |
| +1 :green_heart: | asflicense | 1m 2s | | The patch does not generate ASF License warnings. |
| | | 249m 46s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hdfs.server.federation.router.TestRouterRPCMultipleDestinationMountTableResolver |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/9/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5497 |
| Optional Tests | dupname asflicense mvnsite codespell detsecrets markdownlint compile javac javadoc mvninstall unit shadedclient spotbugs checkstyle |
| uname | Linux 0194e3d5df91 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 3e54acc68aa335eec6c3a3f554e5650c88838ec4 |
| Default Java | Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/9/testReport/ |
| Max. process+thread count | 2461 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-rbf U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/9/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] virajjasani commented on pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#issuecomment-1488984194
Thanks for the reviews sir @ayushtkn @goiri
Could you please help merge this PR if things look good?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] ayushtkn commented on pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#issuecomment-1573357545
@MayankSinghParmar You need to share some more pointers, may be you can subscribe to hdfs-dev mailing list and drop a mail over there and folks would be able to help
https://hadoop.apache.org/mailing_lists.html
Wild Guess:
If it is related to Centralised Cache Management, Can start from here: https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/CacheAdmin.java
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] virajjasani commented on pull request #5497: HDFS-16959. RBF: state store cache loading metrics
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#issuecomment-1476821444
@ayushtkn @goiri could you please review this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#issuecomment-1477302892
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 34s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +0 :ok: | markdownlint | 0m 0s | | markdownlint was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 16m 12s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 26m 41s | | trunk passed |
| +1 :green_heart: | compile | 26m 3s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | compile | 23m 7s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | checkstyle | 3m 40s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 44s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 12s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 1m 58s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 17s | | trunk passed |
| +1 :green_heart: | shadedclient | 21m 18s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 26s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 39s | | the patch passed |
| +1 :green_heart: | compile | 25m 12s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javac | 25m 12s | | the patch passed |
| +1 :green_heart: | compile | 22m 55s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | javac | 22m 55s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 3m 35s | | the patch passed |
| +1 :green_heart: | mvnsite | 2m 42s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 7s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 1m 59s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 21s | | the patch passed |
| +1 :green_heart: | shadedclient | 21m 2s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 48s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 21m 9s | | hadoop-hdfs-rbf in the patch passed. |
| +1 :green_heart: | asflicense | 1m 3s | | The patch does not generate ASF License warnings. |
| | | 259m 14s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5497 |
| Optional Tests | dupname asflicense mvnsite codespell detsecrets markdownlint compile javac javadoc mvninstall unit shadedclient spotbugs checkstyle |
| uname | Linux 8fe75f162719 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / c2d83283337a997559552ce5e4dde56c90945ee6 |
| Default Java | Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/3/testReport/ |
| Max. process+thread count | 3122 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-rbf U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/3/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #5497: HDFS-16959. RBF: state store cache loading metrics
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#issuecomment-1477101526
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 44s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +0 :ok: | markdownlint | 0m 0s | | markdownlint was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 15m 47s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 27m 46s | | trunk passed |
| +1 :green_heart: | compile | 24m 28s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | compile | 21m 51s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | checkstyle | 4m 2s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 44s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 7s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 1m 53s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 39s | | trunk passed |
| +1 :green_heart: | shadedclient | 24m 49s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 29s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 40s | | the patch passed |
| +1 :green_heart: | compile | 25m 36s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javac | 25m 36s | | the patch passed |
| +1 :green_heart: | compile | 22m 48s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | javac | 22m 48s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 3m 58s | | the patch passed |
| +1 :green_heart: | mvnsite | 2m 43s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 6s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 1m 59s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 46s | | the patch passed |
| +1 :green_heart: | shadedclient | 24m 9s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 19m 3s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 21m 1s | | hadoop-hdfs-rbf in the patch passed. |
| +1 :green_heart: | asflicense | 0m 55s | | The patch does not generate ASF License warnings. |
| | | 265m 25s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5497 |
| Optional Tests | dupname asflicense mvnsite codespell detsecrets markdownlint compile javac javadoc mvninstall unit shadedclient spotbugs checkstyle |
| uname | Linux fe6373c1cf41 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 12df4fb32fd6f473965e18caeb171eb6fff147e1 |
| Default Java | Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/1/testReport/ |
| Max. process+thread count | 2568 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-rbf U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/1/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] virajjasani commented on pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#issuecomment-1483884585
For the test, if we want to be accurate about `mountTableCacheLoadAvgTimeMs`, there is a better way too: we can use `FakeTimer` impl and inject it to `CachedRecordStore` by retrieving state store cache list `cachesToUpdateInternal` from the state store service.
I think this might be overkill for the current Jira though. If you like this idea, I can take it up as separate task, but for prod, we are not achieving anything and maybe this is too much for tests as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] virajjasani commented on a diff in pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#discussion_r1148404105
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreDriverBase.java:
##########
@@ -574,6 +580,38 @@ private static Map<String, Class<?>> getFields(BaseRecord record) {
return getters;
}
+ public long getMountTableCacheLoadSamples(StateStoreDriver driver) throws IOException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ return mountTableCache.lastStat().numSamples();
+ }
+
+ private static MutableRate getMountTableCache(StateStoreDriver driver) throws IOException {
+ StateStoreMetrics metrics = stateStore.getMetrics();
+ final Query<MountTable> query = new Query<>(MountTable.newInstance());
+ driver.getMultiple(MountTable.class, query);
+ final Map<String, MutableRate> cacheLoadMetrics = metrics.getCacheLoadMetrics();
+ final MutableRate mountTableCache = cacheLoadMetrics.get("CacheMountTableLoad");
+ assertNotNull("CacheMountTableLoad should be present in the state store metrics",
+ mountTableCache);
+ return mountTableCache;
+ }
+
+ public void testCacheLoadMetrics(StateStoreDriver driver, long numRefresh)
+ throws IOException, IllegalArgumentException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ // CacheMountTableLoadNumOps
+ final long mountTableCacheLoadNumOps = getMountTableCacheLoadSamples(driver);
+ assertEquals("Num of samples collected should match", numRefresh, mountTableCacheLoadNumOps);
+ // CacheMountTableLoadAvgTime ms
+ final double mountTableCacheLoadAvgTimeMs = mountTableCache.lastStat().mean();
+ // 2 seconds is a high enough value for the test, hence we expect mount table cache
+ // with very few entries to be loaded by this time duration, hence not have this test result
+ // show flaky behavior.
+ assertTrue(
+ "Mean time duration for cache load is expected to be less than 2000 ms. Actual value: "
+ + mountTableCacheLoadAvgTimeMs, mountTableCacheLoadAvgTimeMs < 2000d);
+ }
Review Comment:
Oh for prod/test clusters, I think we should be good with ms unit (hopefully) because even with few entries I see at least some value for sure and that is always > 0 so I am sure we should be good there. But yes as you suggested, I would happy to come back here and change unit or make it dynamic if things get wrong (false positive for metric) in future.
For test also, I just did more than 10 runs for each state store impl tests: fs, dfs, zk. And we definitely have values greater than 0 but one time I saw just 0.01d too hence I wonder Jenkins might get 0.0 too. Hence, let me go with your suggestion for test:
> If only in test: May be have the default value as -1 and we can assert it isn't -1 and we should be sorted, if it is -1 then things didin't work the way we wanted.
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] virajjasani commented on a diff in pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#discussion_r1148317954
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreDriverBase.java:
##########
@@ -574,6 +580,38 @@ private static Map<String, Class<?>> getFields(BaseRecord record) {
return getters;
}
+ public long getMountTableCacheLoadSamples(StateStoreDriver driver) throws IOException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ return mountTableCache.lastStat().numSamples();
+ }
+
+ private static MutableRate getMountTableCache(StateStoreDriver driver) throws IOException {
+ StateStoreMetrics metrics = stateStore.getMetrics();
+ final Query<MountTable> query = new Query<>(MountTable.newInstance());
+ driver.getMultiple(MountTable.class, query);
+ final Map<String, MutableRate> cacheLoadMetrics = metrics.getCacheLoadMetrics();
+ final MutableRate mountTableCache = cacheLoadMetrics.get("CacheMountTableLoad");
+ assertNotNull("CacheMountTableLoad should be present in the state store metrics",
+ mountTableCache);
+ return mountTableCache;
+ }
+
+ public void testCacheLoadMetrics(StateStoreDriver driver, long numRefresh)
+ throws IOException, IllegalArgumentException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ // CacheMountTableLoadNumOps
+ final long mountTableCacheLoadNumOps = getMountTableCacheLoadSamples(driver);
+ assertEquals("Num of samples collected should match", numRefresh, mountTableCacheLoadNumOps);
+ // CacheMountTableLoadAvgTime ms
+ final double mountTableCacheLoadAvgTimeMs = mountTableCache.lastStat().mean();
+ // 2 seconds is a high enough value for the test, hence we expect mount table cache
+ // with very few entries to be loaded by this time duration, hence not have this test result
+ // show flaky behavior.
+ assertTrue(
+ "Mean time duration for cache load is expected to be less than 2000 ms. Actual value: "
+ + mountTableCacheLoadAvgTimeMs, mountTableCacheLoadAvgTimeMs < 2000d);
+ }
Review Comment:
That also seems good, either way I think it's fine. The values I have seen have been less than 5 ms so far hence crossing 2000 ms seems like really big deal, even for Jenkins CI, WDYT?
But yeah on the other hand, `mountTableCacheLoadAvgTimeMs > 0d` might also be good enough.
Shall we do `0 < mountTableCacheLoadAvgTimeMs < 2000`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] ayushtkn commented on a diff in pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on code in PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#discussion_r1148330831
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreDriverBase.java:
##########
@@ -574,6 +580,38 @@ private static Map<String, Class<?>> getFields(BaseRecord record) {
return getters;
}
+ public long getMountTableCacheLoadSamples(StateStoreDriver driver) throws IOException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ return mountTableCache.lastStat().numSamples();
+ }
+
+ private static MutableRate getMountTableCache(StateStoreDriver driver) throws IOException {
+ StateStoreMetrics metrics = stateStore.getMetrics();
+ final Query<MountTable> query = new Query<>(MountTable.newInstance());
+ driver.getMultiple(MountTable.class, query);
+ final Map<String, MutableRate> cacheLoadMetrics = metrics.getCacheLoadMetrics();
+ final MutableRate mountTableCache = cacheLoadMetrics.get("CacheMountTableLoad");
+ assertNotNull("CacheMountTableLoad should be present in the state store metrics",
+ mountTableCache);
+ return mountTableCache;
+ }
+
+ public void testCacheLoadMetrics(StateStoreDriver driver, long numRefresh)
+ throws IOException, IllegalArgumentException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ // CacheMountTableLoadNumOps
+ final long mountTableCacheLoadNumOps = getMountTableCacheLoadSamples(driver);
+ assertEquals("Num of samples collected should match", numRefresh, mountTableCacheLoadNumOps);
+ // CacheMountTableLoadAvgTime ms
+ final double mountTableCacheLoadAvgTimeMs = mountTableCache.lastStat().mean();
+ // 2 seconds is a high enough value for the test, hence we expect mount table cache
+ // with very few entries to be loaded by this time duration, hence not have this test result
+ // show flaky behavior.
+ assertTrue(
+ "Mean time duration for cache load is expected to be less than 2000 ms. Actual value: "
+ + mountTableCacheLoadAvgTimeMs, mountTableCacheLoadAvgTimeMs < 2000d);
+ }
Review Comment:
If it more than 0, your metrics is getting captured and propogated, we are fine with that
if I debug this test, it would fail only here, moreover why introduce a scope of a flaky test which can be dodged
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#issuecomment-1483941977
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 36s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +0 :ok: | markdownlint | 0m 0s | | markdownlint was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 16m 3s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 25m 36s | | trunk passed |
| +1 :green_heart: | compile | 23m 4s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | compile | 20m 25s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | checkstyle | 3m 45s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 42s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 10s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 2m 7s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 14s | | trunk passed |
| +1 :green_heart: | shadedclient | 21m 4s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 28s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 34s | | the patch passed |
| +1 :green_heart: | compile | 22m 32s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javac | 22m 32s | | the patch passed |
| +1 :green_heart: | compile | 20m 55s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | javac | 20m 55s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 3m 37s | | the patch passed |
| +1 :green_heart: | mvnsite | 2m 38s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 1s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 2m 5s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 23s | | the patch passed |
| +1 :green_heart: | shadedclient | 21m 27s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 18s | | hadoop-common in the patch passed. |
| -1 :x: | unit | 20m 52s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/16/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) | hadoop-hdfs-rbf in the patch passed. |
| +1 :green_heart: | asflicense | 1m 1s | | The patch does not generate ASF License warnings. |
| | | 247m 15s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hdfs.server.federation.router.TestRouterRPCMultipleDestinationMountTableResolver |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/16/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5497 |
| Optional Tests | dupname asflicense mvnsite codespell detsecrets markdownlint compile javac javadoc mvninstall unit shadedclient spotbugs checkstyle |
| uname | Linux fd1f0bfa708e 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / f85728166c608a84742fc383bce1d2370621a74a |
| Default Java | Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/16/testReport/ |
| Max. process+thread count | 2541 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-rbf U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/16/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] MayankSinghParmar commented on pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "MayankSinghParmar (via GitHub)" <gi...@apache.org>.
MayankSinghParmar commented on PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#issuecomment-1573496151
> @MayankSinghParmar You need to share some more pointers, may be you can subscribe to hdfs-dev mailing list and drop a mail over there and folks would be able to help
>
> https://hadoop.apache.org/mailing_lists.html
>
> Wild Guess:
> If it is related to Centralised Cache Management, Can start from here: https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/CacheAdmin.java
Thanks ayush for the guidance
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#issuecomment-1477317289
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 46s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +0 :ok: | markdownlint | 0m 0s | | markdownlint was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 16m 40s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 29m 55s | | trunk passed |
| +1 :green_heart: | compile | 25m 1s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | compile | 21m 39s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | checkstyle | 3m 54s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 27s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 54s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 1m 45s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 10s | | trunk passed |
| +1 :green_heart: | shadedclient | 24m 8s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 24s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 36s | | the patch passed |
| +1 :green_heart: | compile | 24m 26s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javac | 24m 26s | | the patch passed |
| +1 :green_heart: | compile | 21m 36s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | javac | 21m 36s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 3m 48s | | the patch passed |
| +1 :green_heart: | mvnsite | 2m 27s | | the patch passed |
| +1 :green_heart: | javadoc | 1m 47s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 1m 46s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 21s | | the patch passed |
| +1 :green_heart: | shadedclient | 24m 3s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 20s | | hadoop-common in the patch passed. |
| -1 :x: | unit | 21m 52s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/4/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) | hadoop-hdfs-rbf in the patch passed. |
| +1 :green_heart: | asflicense | 0m 52s | | The patch does not generate ASF License warnings. |
| | | 262m 20s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hdfs.server.federation.router.TestRouterRPCMultipleDestinationMountTableResolver |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/4/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5497 |
| Optional Tests | dupname asflicense mvnsite codespell detsecrets markdownlint compile javac javadoc mvninstall unit shadedclient spotbugs checkstyle |
| uname | Linux 657f21866793 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / af9c23d96579681ed83348f90696ba5e57d1bbd3 |
| Default Java | Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/4/testReport/ |
| Max. process+thread count | 2442 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-rbf U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5497/4/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] virajjasani commented on a diff in pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#discussion_r1142803647
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreDriverBase.java:
##########
@@ -574,6 +578,15 @@ private static Map<String, Class<?>> getFields(BaseRecord record) {
return getters;
}
+ public void testCacheLoadMetrics(StateStoreDriver driver, int numRefresh)
+ throws IOException, IllegalArgumentException {
+ StateStoreMetrics metrics = stateStore.getMetrics();
+ final Query<MountTable> query = new Query<>(MountTable.newInstance());
+ driver.getMultiple(MountTable.class, query);
+ assertEquals(numRefresh,
Review Comment:
The only problem with time duration check is that it could result in flaky tests in future. Is numOps alone fine?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] virajjasani commented on a diff in pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#discussion_r1146494308
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreZK.java:
##########
@@ -206,4 +206,13 @@ public void testFetchErrors()
stateStoreDriver.setEnableConcurrent(true);
testFetchErrors(stateStoreDriver);
}
+
+ @Test
+ public void testCacheLoadMetrics() throws IOException {
+ getStateStoreService().refreshCaches(true);
+ getStateStoreService().refreshCaches(true);
+ getStateStoreService().refreshCaches(true);
+ testCacheLoadMetrics(getStateStoreDriver(), 4);
Review Comment:
Ah yes, I updated it to increment on whatever is the current num of loads. Thanks for pointing this out Ayush!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] ayushtkn commented on a diff in pull request #5497: HDFS-16959. RBF: State store cache loading metrics
Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on code in PR #5497:
URL: https://github.com/apache/hadoop/pull/5497#discussion_r1148316504
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/driver/TestStateStoreDriverBase.java:
##########
@@ -574,6 +580,38 @@ private static Map<String, Class<?>> getFields(BaseRecord record) {
return getters;
}
+ public long getMountTableCacheLoadSamples(StateStoreDriver driver) throws IOException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ return mountTableCache.lastStat().numSamples();
+ }
+
+ private static MutableRate getMountTableCache(StateStoreDriver driver) throws IOException {
+ StateStoreMetrics metrics = stateStore.getMetrics();
+ final Query<MountTable> query = new Query<>(MountTable.newInstance());
+ driver.getMultiple(MountTable.class, query);
+ final Map<String, MutableRate> cacheLoadMetrics = metrics.getCacheLoadMetrics();
+ final MutableRate mountTableCache = cacheLoadMetrics.get("CacheMountTableLoad");
+ assertNotNull("CacheMountTableLoad should be present in the state store metrics",
+ mountTableCache);
+ return mountTableCache;
+ }
+
+ public void testCacheLoadMetrics(StateStoreDriver driver, long numRefresh)
+ throws IOException, IllegalArgumentException {
+ final MutableRate mountTableCache = getMountTableCache(driver);
+ // CacheMountTableLoadNumOps
+ final long mountTableCacheLoadNumOps = getMountTableCacheLoadSamples(driver);
+ assertEquals("Num of samples collected should match", numRefresh, mountTableCacheLoadNumOps);
+ // CacheMountTableLoadAvgTime ms
+ final double mountTableCacheLoadAvgTimeMs = mountTableCache.lastStat().mean();
+ // 2 seconds is a high enough value for the test, hence we expect mount table cache
+ // with very few entries to be loaded by this time duration, hence not have this test result
+ // show flaky behavior.
+ assertTrue(
+ "Mean time duration for cache load is expected to be less than 2000 ms. Actual value: "
+ + mountTableCacheLoadAvgTimeMs, mountTableCacheLoadAvgTimeMs < 2000d);
+ }
Review Comment:
Isn't just asserting this value ain't zero enough?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org