Thank you very much Yiqun and Marton for the feedback/useful comments. Please find my responses below.
@Yiqun, As per the discussions, key deletion tasks have been implemented and the basic functionality of #listKeys works as expected and will be improving it further after merging the feature to master branch. Also, I've created a follow-up umbrella jira for further optimizations/improvement tasks. @Marton, [1] backward compatibility: No backward compatibility issues. S3 works well as the feature code sits behind the ozone.om.metadata.layout feature flag. Yes, same normalization behavior as before. Even with feature turned ON the normalization behaviour is the same, as the configuration "ozone.om.enable.filesystem.paths" should be set to "true". [2] performance: When it is turned OFF then there is zero performance penalty because the existing functionality is not changed. With the feature flag turned OFF, master code will create intermediate directories during create. When it is turned ON the prefix code also does a similar approach during create. But, surely we will be focusing on the performance path post merge and improving it further. [3] documentation: Done. HDDS-5067. Thank you for the useful review comments. [4] security: As you have pointed out, everything works as earlier and there are no security implications because of the feature. [5] branch merging" checklist: I've updated the feature merge wiki page <https://cwiki.apache.org/confluence/display/OZONE/FileSystem+Optimizations+-+HDDS-2939>. Thanks! [6] >>> I assume you planned to write "not blockers" in the first line I’m really sorry for the typo. Yes, "those are not blockers for the merge". All the in progress tasks have been implemented now. [7] >>> We can communicate that the feature is still in alpha phase Yes, we have covered the basic functionalities and #listKeys basic part works fine. We have added unit test cases and acceptance test cases to ensure stability and will be actively working to improve it further after the merge. The feature is disabled by default and won’t affect the stability of the master branch code. @Yiqun, @Marton Welcome suggestions/feedback. Thanks again for your time and your suggestions in shaping up this feature :-) Thanks, Rakesh On Tue, Apr 6, 2021 at 2:05 PM Elek, Marton <e...@apache.org> wrote: > > Thank you very much to start the discussion Rakesh (and thanks to work > on this.) > > As this is something which can greatly improve the performance for the > FS use-cases, I am really exited. > > To make sure that we can guarantee the stability of the master branch, I > would recommend to fill the "branch merging" checklist: > > https://cwiki.apache.org/confluence/display/OZONE/Merging+branches > > It would also provide additional information for all the contributors > who would prefer to test this feature before/during the vote. > > I am especially interested about: > > 1. backward compatibility: if I understand well, it's an optional > feature, so we can use s3 interface as before. But what about if we turn > this feature off, what can we expect from the s3 interface? Do we have > exactly the same normalization behavior as before or it's changed (I > have no problem with any way, as it can be turned on and off, but curious). > > 2. performance: this is already shared on the wiki page, and it's > awesome. One minor question: as far as I understood we have slightly > more overhead on the normal path (we should update / query multiple > rocksdb tables). I don't assume significant slowness there, but do we > have any measurements / numbers related to normal key creation vs master? > > 3. documentation: It would be great to have at least a basic level > documentation as part of 'hadoop-hdds/docs'. It would help us to further > test it in different environments. > > 4. security: I am interested if there are any security implication of > security checks are modified (I assume everything works as before, but > would be great to have confirmation / more information) > > > > > *Ozone wiki page:* > > > > > https://cwiki.apache.org/confluence/display/OZONE/FileSystem+Optimizations+-+HDDS-2939 > > > Thanks to share all these information. I found that some of the > sub-pages have confusing URLs: > > https://cwiki.apache.org/confluence/display/OZONE/Configurations > https://cwiki.apache.org/confluence/display/OZONE/Documents > > These pages are related to the branch but URL may suggest that these are > generic, Ozone related information. > > To fix this, I merged all the subpages to one page and moved the page > under the "Merging branch subsection" to have a consistent structure. > > > > > There are few ongoing sub-tasks, IMHO they are blockers for merge. > > > So, I feel this branch is ready to merge into master branch. > > > I am not sure if I understand these two sentences together, I assume you > planned to write "not blockers" in the first line (two lines seems to > have opposite meaning without this assumption). > > Let me add some related thoughts: There are two implications/risks of > merging something to the master: > > 1. Master can become more unstable > 2. Master will be delivered in the next release to the users > > > 1. seems to be fine, as it can be turned off, the risk is low, and we > can further decrease it with double-checking the stability based on the > mentioned checklist. > > Second is more interesting and I am really interested about your > opinions: if we have a new feature on master, it will be part of the new > release. If the feature is not functional-ready, yet (eg. list keys are > not working, yet), users who turn it on may be confused, as instead of a > new (alpha-) feature they may get an inconsistent / faulty (?) behavior. > > We can communicate that the feature is still in alpha phase, but we need > some level of functional completeness (IMHO). > > As an example: SCM-HA is merged with completeness of security > implementation, but SCM-HA raft ring for non-secure clusters can be > started to test and functional complete. And we clearly communicate that > security can not be used (yet). > > What do you think? > > > > Thanks again to start this discussion, (and if any help is required for > the merge related to build / ci / testing, I would be happy to help / > support it) > > Marton > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@ozone.apache.org > For additional commands, e-mail: dev-h...@ozone.apache.org > >
