satishkotha commented on a change in pull request #1274: [HUDI-571] Add
'commits show archived' command to CLI
URL: https://github.com/apache/incubator-hudi/pull/1274#discussion_r370893819
##########
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java
##########
@@ -289,11 +288,8 @@ public ConsistencyGuardConfig getConsistencyGuardConfig()
{
*
* @return Active commit timeline
*/
- public synchronized HoodieArchivedTimeline getArchivedTimeline() {
- if (archivedTimeline == null) {
- archivedTimeline = new HoodieArchivedTimeline(this);
- }
- return archivedTimeline;
+ public synchronized HoodieArchivedTimeline getArchivedTimeline(String
startTs, String endTs) {
Review comment:
archivedTimeline as instance variable does not make sense because we are
creating archive timeline for small time window (depending on command line
arguments). So we will create mutiple archivedTimelines when someone is
paginating through archived commit files. null check prevents us from doing
that.
In the long term, when we have lazy loading, we can consider creating one
instance of ArchivedTimeline that stores all metadata. we can bring back
instance variable at that time. Until we have that, this does not make sense.
Let me know if you think there is a better way to organize this.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
