Skip to content

Conversation

@cchenax
Copy link
Contributor

@cchenax cchenax commented Mar 16, 2021

What changes were proposed in this pull request?

add keyOffset in the key info result

What is the link to the Apache JIRA

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

How was this patch tested?

example:sh key info /vol1/bucket1/key3

{
  "volumeName" : "vol1",
  "bucketName" : "bucket1",
  "name" : "key3",
  "dataSize" : 644930593,
  "creationTime" : "2021-04-20T09:22:15.206Z",
  "modificationTime" : "2021-04-20T09:22:20.869Z",
  "replicationType" : "RATIS",
  "replicationFactor" : 3,
  "ozoneKeyLocations" : [ {
    "containerID" : 3,
    "localID" : 107544261427200003,
    "length" : 268435456,
    "offset" : 0,
    "keyOffset" : 0
  }, {
    "containerID" : 4,
    "localID" : 107544261427200004,
    "length" : 268435456,
    "offset" : 0,
    "keyOffset" : 268435456
  }, {
    "containerID" : 5,
    "localID" : 107544261427200005,
    "length" : 108059681,
    "offset" : 0,
    "keyOffset" : 536870912
  } ],
  "metadata" : { },
  "fileEncryptionInfo" : null
}

@adoroszlai adoroszlai changed the title Display key offset for each block in command key info HDDS-4983. Display key offset for each block in command key info Mar 24, 2021
Copy link
Contributor

@cku328 cku328 left a comment

Choose a reason for hiding this comment

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

Thanks @cchenax for working on this.
I have some suggestions for checkstyle not to pass.

/**
* KeyOffset of this key.
*/
private final long KeyOffset;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private final long KeyOffset;
private final long keyOffset;

use lower camel case: KeyOffset -> keyOffset

*/
public OzoneKeyLocation(long containerID, long localID,
long length, long offset) {
long length, long offset) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: seems like an unnecessary adjustment.

/**
* Returns the KeyOffset of this Key.
*/
public long getKeyOffset() { return KeyOffset; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public long getKeyOffset() { return KeyOffset; }
public long getKeyOffset() {
return keyOffset;
}

blocks should have line breaks.

@elek
Copy link
Member

elek commented Apr 8, 2021

For the records: jira issue has a conversation about improving the initial implementation:

image

@cchenax
Copy link
Contributor Author

cchenax commented Apr 8, 2021

For the records: jira issue has a conversation about improving the initial implementation:

image

yes,I will improve it.

Copy link
Member

@elek elek left a comment

Choose a reason for hiding this comment

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

Thanks the patch @cchenax

Overall it looks good to me, I checked the class and it's used only on the client side, safe to modify.

I have one question about simplifying the loop.

@cchenax
Copy link
Contributor Author

cchenax commented Apr 26, 2021

Thanks the patch @cchenax

Overall it looks good to me, I checked the class and it's used only on the client side, safe to modify.

I have one question about simplifying the loop.

ok,this way is good,thank you very much.

Copy link
Member

@elek elek left a comment

Choose a reason for hiding this comment

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

+1, thanks the update @cchenax

@elek elek merged commit 10312fb into apache:master Apr 29, 2021
errose28 added a commit to errose28/ozone that referenced this pull request May 4, 2021
…ing-upgrade-master-merge2

* upstream/master: (56 commits)
  HDDS-2212. Genconf tool should generate config files for secure clust… (apache#1788)
  HDDS-5166. Remove duplicate assignment of OZONE_OPTS for freon and sh (apache#2195)
  Revert "HDDS-5144. Create github check to alert when dependency tree is changed (apache#2177)"
  HDDS-4983. Display key offset for each block in command key info (apache#2051)
  HDDS-5144. Create github check to alert when dependency tree is changed (apache#2177)
  HDDS-4585. Support bucket acl operation in S3g (apache#1701)
  HDDS-5153. Decommissioning a dead node should complete immediately (apache#2190)
  HDDS-5147. Intermittent test failure in TestContainerDeletionChoosingPolicy#testRandomChoosingPolicy (apache#2188)
  HDDS-5152. Fix Suggested leader in Client. (apache#2189)
  HDDS-5148. Bump ratis version to 2.1.0-ff8aa66-SNAPSHOT (apache#2184)
  HDDS-4515. Datanodes should be able to persist and load CRL (apache#2181)
  HDDS-5060. [SCM HA Security] Make InterSCM grpc channel secure. (apache#2187)
  HDDS-5051. Ensure failover to suggested leader if any for NotLeaderException. (apache#2141)
  HDDS-5127. Fix getServiceList when SCM HA is enabled (apache#2173)
  HDDS-4889. Add simple CI check for docs (apache#2156)
  HDDS-5131. Use timeout in github actions (apache#2176)
  HDDS-5103. Fix Install Snapshot Mechanism in SCMStateMachine. (apache#2155)
  HDDS-5124. Use OzoneConsts.OZONE_TIME_ZONE instead of "GMT" (apache#2166)
  HDDS-5047. Refactor Pipeline to use ReplicationConfig instead of factor/type (apache#2096)
  HDDS-5083. Bump version of common-compress (apache#2139)
  ...

Conflicts:
	hadoop-hdds/common/pom.xml
	hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java
	hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java
	hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java
	hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
	hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManager.java
	hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMStorageConfig.java
	hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
	hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestSCMNodeManager.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java
	hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMStorage.java
	hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
	hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconStorageContainerManagerFacade.java
errose28 added a commit to errose28/ozone that referenced this pull request May 13, 2021
…k-in-auth

* HDDS-3698-nonrolling-upgrade: (57 commits)
  Fix compilation errors afte merge Update javassist in recon pom Fix changes introduced in merge that failed TestSCMNodeManager upgrade tests Fix checkstyle Fix intermittent test failure TestSCMNodeManager#testSetNodeOpStateAndCommandFired after merge Skip scm init default layout version in TestOzoneConfigurationFields
  HDDS-2212. Genconf tool should generate config files for secure clust… (apache#1788)
  HDDS-5166. Remove duplicate assignment of OZONE_OPTS for freon and sh (apache#2195)
  Revert "HDDS-5144. Create github check to alert when dependency tree is changed (apache#2177)"
  HDDS-4983. Display key offset for each block in command key info (apache#2051)
  HDDS-5144. Create github check to alert when dependency tree is changed (apache#2177)
  HDDS-4585. Support bucket acl operation in S3g (apache#1701)
  HDDS-5153. Decommissioning a dead node should complete immediately (apache#2190)
  HDDS-5147. Intermittent test failure in TestContainerDeletionChoosingPolicy#testRandomChoosingPolicy (apache#2188)
  HDDS-5152. Fix Suggested leader in Client. (apache#2189)
  HDDS-5148. Bump ratis version to 2.1.0-ff8aa66-SNAPSHOT (apache#2184)
  HDDS-4515. Datanodes should be able to persist and load CRL (apache#2181)
  HDDS-5060. [SCM HA Security] Make InterSCM grpc channel secure. (apache#2187)
  HDDS-5051. Ensure failover to suggested leader if any for NotLeaderException. (apache#2141)
  HDDS-5127. Fix getServiceList when SCM HA is enabled (apache#2173)
  HDDS-4889. Add simple CI check for docs (apache#2156)
  HDDS-5131. Use timeout in github actions (apache#2176)
  HDDS-5103. Fix Install Snapshot Mechanism in SCMStateMachine. (apache#2155)
  HDDS-5124. Use OzoneConsts.OZONE_TIME_ZONE instead of "GMT" (apache#2166)
  HDDS-5047. Refactor Pipeline to use ReplicationConfig instead of factor/type (apache#2096)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants