Skip to content

Comments

HDDS-8904. Fix Snapdiff output for key renames#5177

Merged
hemantk-12 merged 3 commits intoapache:masterfrom
swamirishi:HDDS-8904_PR
Aug 31, 2023
Merged

HDDS-8904. Fix Snapdiff output for key renames#5177
hemantk-12 merged 3 commits intoapache:masterfrom
swamirishi:HDDS-8904_PR

Conversation

@swamirishi
Copy link
Contributor

@swamirishi swamirishi commented Aug 12, 2023

What changes were proposed in this pull request?

Currently snapdiff shows modify entry for keys which have undergone a series of rename and gets renamed back to the same name. HDFS currently shows a rename for such cases.
There is a difference b/w Ozone & HDFS output:
HDFS Ouput
Difference between snapshot s0 and snapshot s1 under directory /:
M .
R ./foo -> ./foo

Ozone Output before change
Difference between snapshot: snap1 and snapshot: snap2
M ./dir1
M ./dir1/k12

Ozone Output after change
Difference between snapshot: snap1 and snapshot: snap2
R ./dir1/k12 -> ./dir1/k12
M ./dir1

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-8904

How was this patch tested?

Unit tests and Integration tests

@umamaheswararao
Copy link
Contributor

@hemantk-12 could you please help to review this?

Copy link
Contributor

@hemantk-12 hemantk-12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch @swamirishi.

I am still not not clear why this change is needed because initial change was done as part HDDS-8841. Is it related to FSO bucket? Or Legacy and OBS as well? It would be great if you could add an examples of difference between before and after the change in the description. Or difference between Ozone and HDFS.

@swamirishi
Copy link
Contributor Author

swamirishi commented Aug 29, 2023

Thanks for the patch @swamirishi.

I am still not not clear why this change is needed because initial change was done as part HDDS-8841. Is it related to FSO bucket? Or Legacy and OBS as well? It would be great if you could add an examples of difference between before and after the change in the description. Or difference between Ozone and HDFS.

@hemantk-12 Added this in the description

@aswinshakil aswinshakil added the snapshot https://issues.apache.org/jira/browse/HDDS-6517 label Aug 29, 2023

@Override
public byte[] toPersistedFormat(Boolean object) {
return object ? new byte[]{TRUE} : new byte[]{FALSE};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: create constants for new byte[]{TRUE} and new byte[]{FALSE};

Copy link
Contributor

@hemantk-12 hemantk-12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @swamirishi for adding examples in the descriptions.

I did comparison between current and after the change and below is the diff for different scenarios.

  1. Renamed to same key for non-dir keys: keya -> keyb -> keyc -> keya
    Before the change:
M       ./keya

After the change:

R       ./keya -> ./keya
  1. Renamed to same key for key in a dir: dir1/key1 -> dir1/key2 -> dir1/key1
    Before the change:
M       ./dir1
M       ./dir1/key1

After the change:

R       ./dir1/key1 -> ./dir1/key1
M       ./dir1
  1. Key is overridden and renamed key: non dir key
    Before the change:
R       ./key11 -> ./key12
M       ./key11

After the change:

R       ./key11 -> ./key12
M       ./key11
  1. Key is overridden and renamed key: key with dir
    Before the change:
R       ./dir1/key1 -> ./dir1/key2
M       ./dir1/key1
M       ./dir1

After the change:

R       ./dir1/key1 -> ./dir1/key2
M       ./dir1
M       ./dir1/key1

Based on above testing, change looks good to me.

@hemantk-12 hemantk-12 merged commit e03e9f3 into apache:master Aug 31, 2023
jojochuang pushed a commit to jojochuang/ozone that referenced this pull request Oct 24, 2023
(cherry picked from commit e03e9f3)
Change-Id: I0f440e610ac4897a3c5b7491214b7ad0f64857ec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

snapshot https://issues.apache.org/jira/browse/HDDS-6517

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants