New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[#323] Unity: add advanced dedup support #324

Merged

yong-huang merged 2 commits into master from advanced-dedup

Nov 24, 2020

Contributor

yong-huang commented Sep 21, 2020

Add Advanced Deduplication support for Unity LUNs.

yong-huang requested review from Murray-LIANG and amagicsun

September 21, 2020 08:18

codecov bot commented Sep 21, 2020 •

edited

Loading

Codecov Report

Merging #324 (9d23f7d) into master (e435790) will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #324      +/-   ##
==========================================
+ Coverage   91.70%   91.77%   +0.06%     
==========================================
  Files          99      100       +1     
  Lines       13929    13998      +69     
  Branches     1687     1697      +10     
==========================================
+ Hits        12774    12846      +72     
+ Misses        741      740       -1     
+ Partials      414      412       -2

Impacted Files	Coverage Δ
storops/unity/resource/move_session.py	`100.00% <ø> (ø)`
storops/exception.py	`97.44% <100.00%> (+0.03%)`	⬆️
storops/lib/models.py	`100.00% <100.00%> (ø)`
storops/unity/enums.py	`98.84% <100.00%> (+<0.01%)`	⬆️
storops/unity/resource/lun.py	`94.33% <100.00%> (+0.55%)`	⬆️
storops/unity/resource/pool.py	`100.00% <100.00%> (ø)`
storops/vnx/navi_command.py	`85.71% <0.00%> (+0.71%)`	⬆️
storops/connection/connector.py	`46.90% <0.00%> (+0.88%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e435790...9d23f7d. Read the comment docs.

amagicsun reviewed

View reviewed changes

storops/unity/resource/lun.py

@@ @@ -76,7 +76,8 @@ def make_compression_body(cli=None, @@
                       fastVPParameters=UnityClient.make_body(
                           tieringPolicy=kwargs.get('tiering_policy')),
                       ioLimitParameters=UnityClient.make_body(
-                          ioLimitPolicy=kwargs.get('io_limit_policy')))
+                          ioLimitPolicy=kwargs.get('io_limit_policy')),
+                      isAdvancedDedupEnabled=kwargs.get('is_advanced_dedup_enabled'))

amagicsun Sep 21, 2020

indent

Contributor Author

yong-huang Sep 24, 2020

It is not an indent issue, the level of isAdvancedDedupEnabled is same as ioLimitParameters.

storops/unity/resource/pool.py Outdated

+                  def is_advanced_dedup_enabled(self):
+                      unity_support_matrix = ['450F', '550F', '650F', '380', '480', '680',
+                                              '880', '380F', '480F', '680F', '880F']
+                      return self.verify_advanced_dedup_by_unity_model(unity_support_matrix)

amagicsun Sep 21, 2020

How to check whether it is AFP for 380/480/680/880?

Contributor Author

yong-huang Sep 24, 2020

Check the is_all_flash attribute in self.verify_advanced_dedup_by_unity_model.

Murray-LIANG reviewed

View reviewed changes

storops/unity/resource/pool.py Outdated

@@ @@ -203,6 +206,51 @@ def disk_groups(self): @@
                               dgs[pd.disk_group.get_id()] = [pd]
                       return dgs
+                  @version('<5.1')
+                  def is_compression_enabled(self):

Contributor

Murray-LIANG Sep 21, 2020

Change is_compression_enabled to is_compression_supported? Because cannot enable/disable compression on pool.

Contributor Author

yong-huang Sep 24, 2020

Done.

storops/unity/resource/pool.py Outdated

+                                              'verifyDataReduction')
+                      resp.raise_if_err()
+                      if resp.body.get('content') and resp.body.get('content').get(
+                              'verifyPass') is True:

Contributor

Murray-LIANG Sep 21, 2020

is True is redundant.
You could also return resp.body.get('content') and resp.body.get('content').get('verifyPass').

Contributor Author

yong-huang Sep 24, 2020

Done.

storops/unity/resource/pool.py Outdated

+                  @version('=4.5')  # noqa
+                  def is_advanced_dedup_enabled(self):
+                      unity_support_matrix = ['450F', '550F', '650F']

Contributor

Murray-LIANG Sep 21, 2020

Can we add a decorator for unity_models like version?
So, we can share the code in other places where we need to check the unity model.

Contributor Author

yong-huang Nov 24, 2020

Done, added a UnityModel class to provide model compare operations.

storops/unity/resource/pool.py Outdated

+                      return self.is_all_flash
+                  @version('>=5.1')  # noqa
+                  def is_compression_enabled(self):

Contributor

Murray-LIANG Sep 21, 2020

s/enabled/supported

Contributor Author

yong-huang Sep 24, 2020

Done.

storops/unity/resource/pool.py Outdated

+                      return unity_model in unity_support_matrix and self.is_all_flash
+                  @version('<4.5')
+                  def is_advanced_dedup_enabled(self):

Contributor

Murray-LIANG Sep 21, 2020

s/enabled/supported

Contributor Author

yong-huang Sep 24, 2020

Done.

storops/unity/resource/pool.py Outdated

+                      return False
+                  @version('=4.5')  # noqa
+                  def is_advanced_dedup_enabled(self):

Contributor

Murray-LIANG Sep 21, 2020

s/enabled/supported

Contributor Author

yong-huang Sep 24, 2020

Done.

storops/unity/resource/pool.py Outdated

+                      return self.verify_advanced_dedup_by_unity_model(unity_support_matrix)
+                  @version('=5.0')  # noqa
+                  def is_advanced_dedup_enabled(self):

Contributor

Murray-LIANG Sep 21, 2020

s/enabled/supported

Contributor Author

yong-huang Sep 24, 2020

Done.

storops/unity/resource/pool.py Outdated

+                      return self.verify_advanced_dedup_by_unity_model(unity_support_matrix)
+                  @version('>=5.1')  # noqa
+                  def is_advanced_dedup_enabled(self):

Contributor

Murray-LIANG Sep 21, 2020

s/enabled/supported

Contributor Author

yong-huang Sep 24, 2020

Done.

storops/unity/resource/pool.py Outdated

+                                              'verifyDeduplication')
+                      resp.raise_if_err()
+                      if resp.body.get('content') and resp.body.get('content').get(
+                              'verifyPass') is True:

Contributor

Murray-LIANG Sep 21, 2020

is True is redundant.

Contributor Author

yong-huang Sep 24, 2020

Done.

storops/unity/resource/pool.py Outdated

+                      from storops.unity.resource.system import UnitySystem
+                      unity_system = UnitySystem(cli=self._cli)
+                      unity_model = unity_system.model.split()[-1]
+                      return unity_model in unity_support_matrix and self.is_all_flash

Contributor

Murray-LIANG Sep 21, 2020

is_all_flash is available for OE < 5.0, right?

Contributor Author

yong-huang Sep 24, 2020

Yes.

yong-huang force-pushed the advanced-dedup branch from ba4c8dd to c32db14 Compare

September 23, 2020 07:57

Murray-LIANG reviewed

View reviewed changes

storops/unity/resource/lun.py Outdated

+                              self._cli, self, dest,
+                              is_data_reduction_applied=is_compressed,
+                              is_dest_thin=is_thin,
+                              is_advanced_dedup_enabled=is_advanced_dedup_enabled)

Contributor

Murray-LIANG Sep 23, 2020

is_advanced_dedup_enabled? It should be is_advanced_dedup_applied? Did pep8 pass?

Contributor Author

yong-huang Sep 23, 2020

Please skip this commit, it is not fully tested.

Contributor Author

yong-huang Sep 24, 2020

Fixed.

yong-huang force-pushed the advanced-dedup branch 5 times, most recently from 512c069 to a00ebff Compare

September 27, 2020 09:44

yong-huang added 2 commits

November 19, 2020 18:49


          [#323] Unity: add advanced dedup support

208b147

Add Advanced Deduplication support for Unity LUNs.


          Unity: add UnityModel class

9d23f7d

yong-huang force-pushed the advanced-dedup branch from 0448f92 to 9d23f7d Compare

November 19, 2020 10:50

yong-huang merged commit bd19185 into master

yong-huang deleted the advanced-dedup branch

November 24, 2020 01:50

yong-huang added a commit that referenced this pull request


          Release 1.2.8

1c4e44b

- bd19185 [#323] Unity: add advanced dedup support (#324)
- e435790 Unity: fix fs/nfs/cifs issues (#335)
- d4a2818 VNX: support to specify SP in LUN creation (#334)
- 0437478 [#325] Unity: add parameters for fs/nfs/cifs create and modify (#331)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet