Thanks for working on this feature! +1
Regards Lokesh > On 08-Apr-2021, at 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 >> >> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@ozone.apache.org For additional commands, e-mail: dev-h...@ozone.apache.org
