fapifta commented on pull request #1322: URL: https://github.com/apache/hadoop-ozone/pull/1322#issuecomment-674766274
Hi @avijayanhwx, thank you for starting this work, and posting an inital version of the core code for the upgrade support. I have a few general questions and concerns, also added a few comments after a quick review. In HDFS the layoutversion is a monotonically decreasing number, we chose to use monotonically increasing version numbers, I am unsure why HDFS chose the negative numbers, can there be some hidden considerations, we might go through, before committing to the positive layoutversions and monotonic increase? I haven't found one. Also, in HDFS one layoutversion covers one feature, would it be better to do not let two feetures associated with one layoutversion number? What is the benefit of having two features mapped to the same layout version? I don't feel good about it, though I don't have a specific example yet where it can cause trouble. LayoutVersionManager is implemented in a way that it is fully static. I am unsure if this is a good design, I understand the intent that there has to be only one instance of this per component, and seeing it this way it is a reasonable choice to use static a implementation, but it can fire back later when we want to implement tests that involves changing something in this logic, and we can not freely and easily change the behaviour in tests as I fear, also it can introduce invisible interdependencies later that might be hard to detect/factor out. Implementing it in a non-static way does not seem to cause any drawback, even we can be fine with multiple instances for the same component, as the used values will be anyway hardcoded in the real system. What do you think, I would consider making it non-static, as I think it has more possibilities and less limitations later. ---------------------------------------------------------------- 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: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
