+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 > > > > >
