+1 Thanks Shashi
On Fri, Apr 9, 2021 at 1:39 PM Sadanand Shenoy < sadanand.shenoy4...@gmail.com> wrote: > +1, Thanks for working on this feature. > > Regards, > Sadanand > > On Thu, Apr 8, 2021, 11:05 PM Rakesh Radhakrishnan <rake...@apache.org> > wrote: > > > 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 > > > > > > > > >
