ozone-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Lokesh Jain <lj...@apache.org>
Subject Re: [DISCUSS] - Merge FileSystem Optimizations branch HDDS-2939 to master
Date Fri, 09 Apr 2021 07:04:09 GMT
Thanks for working on this feature!

+1

Regards
Lokesh

> On 08-Apr-2021, at 11:05 PM, Rakesh Radhakrishnan <rakeshr@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 <elek@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-unsubscribe@ozone.apache.org
>> For additional commands, e-mail: dev-help@ozone.apache.org
>> 
>> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ozone.apache.org
For additional commands, e-mail: dev-help@ozone.apache.org


Mime
View raw message