HDDS-1705. Recon: Add estimatedTotalCount to the response of ... #1055
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rs and containers/{id} endpoints
|
/label ozone |
|
@swagle @elek @avijayanhwx Please review when you find time. |
swagle
reviewed
Jul 3, 2019
hadoop-ozone/ozone-recon/src/main/java/org/apache/hadoop/ozone/recon/ReconServer.java
Outdated
Show resolved
Hide resolved
swagle
reviewed
Jul 3, 2019
...con/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ContainerDBServiceProviderImpl.java
Outdated
Show resolved
Hide resolved
swagle
reviewed
Jul 3, 2019
...con/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ContainerDBServiceProviderImpl.java
Outdated
Show resolved
Hide resolved
swagle
reviewed
Jul 3, 2019
| @Override | ||
| public void storeContainerCount(Long count) { | ||
| // Get the current timestamp | ||
| Timestamp now = |
There was a problem hiding this comment.
Any reason to ask sql timestamp vs System.currentMillis()?
There was a problem hiding this comment.
Yes. The assumption here is that the SQL database will be running in UTC timezone as a standard practice for all production databases and we don't want to mess with the timezone by using System.currentMillis().
There was a problem hiding this comment.
But Ozone will rely on JVM timestamp, and since time is relative, as long as we don't change the impl midway this point is moot, isn't it :-)
There was a problem hiding this comment.
At some point in the future, we might show the last updated timestamp in the UI and that is why using SQL timestamp here. As long as this value is only used by the application, we can use System.currentMillis() and that is what we are doing for lastUpdatedTimestamp in ReconInternalSchema.
swagle
reviewed
Jul 3, 2019
...con/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ContainerDBServiceProviderImpl.java
Outdated
Show resolved
Hide resolved
swagle
reviewed
Jul 3, 2019
| }); | ||
|
|
||
| // The following setup is run only once | ||
| if (!setUpIsDone) { |
There was a problem hiding this comment.
@BeforeClass annotated methods are guaranteed to be run once by junit framework.
There was a problem hiding this comment.
Yes, but @BeforeClass annotated methods need to be static and cannot be used in this case since members of the class need to be updated or accessed.
There was a problem hiding this comment.
Also, power mock runner does not apply JUnit ClassRules - powermock/powermock#687
The workaround to use BlockJUnit4ClassRunner.class also doesn't work as expected and causes a lot of test failures. Hence, sticking with this approach for test classes that use power mock runner.
swagle
reviewed
Jul 3, 2019
...ne/ozone-recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerKeyMapperTask.java
Show resolved
Hide resolved
swagle
reviewed
Jul 3, 2019
...zone-recon/src/test/java/org/apache/hadoop/ozone/recon/tasks/TestContainerKeyMapperTask.java
Outdated
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
swagle
reviewed
Jul 9, 2019
...p-ozone/ozone-recon/src/test/java/org/apache/hadoop/ozone/recon/types/GuiceInjectorTest.java
Outdated
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
|
+1 |
|
💔 -1 overall
This message was automatically generated. |
shanthoosh
pushed a commit
to shanthoosh/hadoop
that referenced
this pull request
Oct 15, 2019
…re picked up during next deployment of app Author: Ray Matharu <rmatharu@linkedin.com> Reviewers: Bharath Kumarasubramaniam <bkumaras@linkedin.com>, Jagadish Venkatraman <vjagadish1989@gmail.com> Closes apache#1055 from rmatharu/regex-fix
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…containers and containers/{id} endpoints
This PR adds the following features to Recon
This patch was tested manually by bringing up ozone in a local dev environment and checking whether sqlite instance gets updated with total number of containers as expected. Also, the API responses of two endpoints were tested in a browser with different limits and prevKey combinations.