Skip to content

Conversation

@adoroszlai
Copy link
Contributor

What changes were proposed in this pull request?

DatanodeDetails#toString outputs too much detail for it to be usable in each and every log message related to datanodes. Thus log statements currently have to build their own output (e.g. for list of hosts, etc.), leading to duplication.

This PR replaces toString with less verbose implementation, but keeps the current one for occasional usage as toDebugString.

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

How was this patch tested?

Checked for log messages in TestECContainerRecovery output.

... [Under Replicated Processor] INFO  replication.ReplicationManager (ReplicationManager.java:sendDatanodeCommand(426)) - Sending command [reconstructECContainersCommand: containerID: 1, replicationConfig: EC/ECReplicationConfig{data=3, parity=2, ecChunkSize=1024, codec=rs}, sources: [38ebee1d-b2cd-4e58-9de6-20f391beb291(localhost/127.0.0.1) replicaIndex: 2, 66af460b-94b6-43e2-85e5-afa286f13712(localhost/127.0.0.1) replicaIndex: 3, 10ecf3b6-90a9-4cf6-8e18-633a6d41e838(localhost/127.0.0.1) replicaIndex: 4, a0edd823-1e08-4f0b-905c-87b466267435(localhost/127.0.0.1) replicaIndex: 5], targets: [edc89870-9c73-4040-b6c3-508456f7c48e(localhost/127.0.0.1)], missingIndexes: [1]] for container ContainerInfo{id=#1, state=CLOSED, pipelineID=PipelineID=60cb9ff8-f103-4e1d-8113-1704304fe882, stateEnterTime=2023-01-09T15:35:20.157Z, owner=om1} to edc89870-9c73-4040-b6c3-508456f7c48e(localhost/127.0.0.1)
... [ECContainerReconstructionThread-0] INFO  reconstruction.ECReconstructionCoordinatorTask (ECReconstructionCoordinatorTask.java:run(97)) - Completed ECReconstructionCommand{containerID=1, replication=rs-3-2-1024, missingIndexes=[1], sources={2=38ebee1d-b2cd-4e58-9de6-20f391beb291(localhost/127.0.0.1), 3=66af460b-94b6-43e2-85e5-afa286f13712(localhost/127.0.0.1), 4=10ecf3b6-90a9-4cf6-8e18-633a6d41e838(localhost/127.0.0.1), 5=a0edd823-1e08-4f0b-905c-87b466267435(localhost/127.0.0.1)}, targets={1=edc89870-9c73-4040-b6c3-508456f7c48e(localhost/127.0.0.1)` in 272 ms
... [Over Replicated Processor] INFO  replication.ReplicationManager (ReplicationManager.java:sendDatanodeCommand(426)) - Sending command [deleteContainerCommand: containerID: 1, replicaIndex: 1, force: true] for container ContainerInfo{id=#1, state=CLOSED, pipelineID=PipelineID=60cb9ff8-f103-4e1d-8113-1704304fe882, stateEnterTime=2023-01-09T15:35:20.157Z, owner=om1} to b4e72fba-ed18-4213-b609-5d05054eab1c(localhost/127.0.0.1)
... [Under Replicated Processor] INFO  replication.ReplicationManager (ReplicationManager.java:sendDatanodeCommand(426)) - Sending command [reconstructECContainersCommand: containerID: 2, replicationConfig: EC/ECReplicationConfig{data=3, parity=2, ecChunkSize=1024, codec=rs}, sources: [5f45cafe-8ef6-4bae-8d82-ead9d2bb2384(localhost/127.0.0.1) replicaIndex: 2, 66af460b-94b6-43e2-85e5-afa286f13712(localhost/127.0.0.1) replicaIndex: 3, b4e72fba-ed18-4213-b609-5d05054eab1c(localhost/127.0.0.1) replicaIndex: 4, 5d638143-0bbd-4dd8-931f-e16d2a604d5d(localhost/127.0.0.1) replicaIndex: 5], targets: [edc89870-9c73-4040-b6c3-508456f7c48e(localhost/127.0.0.1)], missingIndexes: [1]] for container ContainerInfo{id=#2, state=CLOSED, pipelineID=PipelineID=410d64cf-d917-4b03-b3a0-dc4fdeb535d7, stateEnterTime=2023-01-09T15:35:58.366Z, owner=om1} to edc89870-9c73-4040-b6c3-508456f7c48e(localhost/127.0.0.1)
... [ECContainerReconstructionThread-1] WARN  reconstruction.ECReconstructionCoordinatorTask (ECReconstructionCoordinatorTask.java:run(100)) - Failed ECReconstructionCommand{containerID=2, replication=rs-3-2-1024, missingIndexes=[1], sources={2=5f45cafe-8ef6-4bae-8d82-ead9d2bb2384(localhost/127.0.0.1), 3=66af460b-94b6-43e2-85e5-afa286f13712(localhost/127.0.0.1), 4=b4e72fba-ed18-4213-b609-5d05054eab1c(localhost/127.0.0.1), 5=5d638143-0bbd-4dd8-931f-e16d2a604d5d(localhost/127.0.0.1)}, targets={1=edc89870-9c73-4040-b6c3-508456f7c48e(localhost/127.0.0.1)` after 2333 ms

Regular CI:
https://github.com/adoroszlai/hadoop-ozone/actions/runs/3875394666

@adoroszlai adoroszlai self-assigned this Jan 9, 2023
@kerneltime
Copy link
Contributor

@xBis7 can you please take a look?

Copy link
Contributor

@sodonnel sodonnel left a comment

Choose a reason for hiding this comment

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

LGTM - I think this is a good change, as the current toString() on datanodeDetails is used in a lot of places and it is very verbose. It makes the logs hard to read when debugging an issue, when all you often want to know is what DN a command is targeted for etc.

@adoroszlai adoroszlai merged commit 4531701 into apache:master Jan 10, 2023
@adoroszlai adoroszlai deleted the HDDS-7753 branch January 10, 2023 20:33
@adoroszlai
Copy link
Contributor Author

Thanks @sodonnel for the review.

jojochuang pushed a commit to jojochuang/ozone that referenced this pull request Feb 6, 2023
…pache#4161)

(cherry picked from commit 4531701)
Change-Id: I897e030c5a2b307879556ea25be53567c1dfe88c
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