Skip to content

HDDS-10271. Fixing SCM start-up logs.#6146

Merged
adoroszlai merged 1 commit into
apache:masterfrom
nandakumar131:HDDS-10271
Feb 2, 2024
Merged

HDDS-10271. Fixing SCM start-up logs.#6146
adoroszlai merged 1 commit into
apache:masterfrom
nandakumar131:HDDS-10271

Conversation

@nandakumar131
Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

  • We are already logging all the config properties during start-up, in addition to that the container balancer config is also logged. We can avoid logging container balancer config during startup.
  • We don't have to print the complete certificate information at the info level. Printing SerialId should be enough.
  • Fix incorrect log message in ContainerStateManager#initialize

What is the link to the Apache JIRA

HDDS-10271

How was this patch tested?

NA

Copy link
Copy Markdown
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @nandakumar131 for the patch, LGTM.

Would be nice to show examples of log messages (before/after) in the PR.

Copy link
Copy Markdown
Contributor

@errose28 errose28 left a comment

Choose a reason for hiding this comment

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

LGTM, but I agree with Attila's idea that before/after log examples in the PR description will help when referencing this change later.

@Galsza
Copy link
Copy Markdown
Contributor

Galsza commented Feb 2, 2024

@nandakumar131 thank you for the patch

Copy link
Copy Markdown
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Added log samples for reference.

updateCachedData(fileName, CAType.ROOT, this::updateCachedRootCAId);

getLogger().info("Added certificate {} from file: {}.", cert,
getLogger().info("Added certificate {} from file: {}.", readCertSerialId,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Original:

2024-02-01 18:05:28,728 [main] INFO security.OMCertificateClient: Added certificate   [0]         Version: 3
         SerialNumber: 9
             IssuerDN: CN=scm-sub@scm,OU=723c6f04-c5f4-42a3-a4d9-2f4a81299889,O=CID-7f90c354-6233-41b6-a586-d8e6dce0e7f6,SERIALNUMBER=2
           Start Date: Thu Feb 01 18:05:27 UTC 2024
           Final Date: Fri Jan 31 18:05:27 UTC 2025
            SubjectDN: CN=om,OU=723c6f04-c5f4-42a3-a4d9-2f4a81299889,O=CID-7f90c354-6233-41b6-a586-d8e6dce0e7f6,SERIALNUMBER=9
           Public Key: RSA Public Key [6a:ae:60:f5:63:c6:92:ce:70:22:a5:bc:f9:db:76:7a:f9:fc:ca:eb],[56:66:d1:a4]
        modulus: b2a9cc8eac14f855cfdbbda5e911d41e33f491c01320322d96cd1fac8b36b0c5a24acde78cc9e545ba204b0bd67d3d4e07d1acba16b4dbaed1ab01b61306012b70a93e2ddd5a08464ad47df998b32e36a0ccac45e59929f68c276e5aba0f733a2d10a3d80bdb8f14b71b894b9364b6259aafd36a0a0b502d6f3424064333b147547888a836fda94d73f2756ceb1d210153bb43e5c9155d2b38aa9851e7220010e5c3940bf029ae9d4e9eec337cd570684751ecdf424407b5649cb141932ec9c17040d514ed77fa363e9df4d3f8b790996a9f6ca2209679dd7cc92a6a76c6c8face588d14f8d7cc7bda1a0fe0360b9d32341120dc60ace8a951d1748fc71a34d5
public exponent: 10001

  Signature Algorithm: SHA256WITHRSA
            Signature: 4db15d4ffc2bd405d9314ba3b5733497534317ab
                       789644867aa4a7ce42e2bbc91019f7e866dcc9ec
                       e56b2ff27e019a3062a13880ee900f1211b3bf8e
                       bbd0952e77c9f0167e2ca1c9ddf6f9db1a090433
                       4133cc9a0986dc622b18a1d0b93351509aec0da2
                       a1ecc1d4492072c91014913de39545530297a8e8
                       bdf78428338e1288018c7d688a7d0be2766b88af
                       b54f02422b85c51c4087b6cfe2a1860d4eb01b91
                       7fadcc566eba8e0ad3580a77cb949157183e1165
                       026d828f2d05cd84949b7a83e74c03b7c616585e
                       01d0506754fd8d59ceefd62e17ea35d078605b52
                       59e79a6d498814de874e7b3feb9d4f4bd5d6e92d
                       4f81d8163ec716dec52df107cd9d4664
       Extensions: 
                       critical(false) 2.5.29.17 value = Sequence
    Tagged [CONTEXT 7] IMPLICIT 
        DER Octet String[4] 

                       critical(true) KeyUsage: 0xb8
 from file: /data/metadata/om/certs/9.crt.

New:

2024-02-02 17:19:13,571 [main] INFO security.OMCertificateClient: Added certificate 4 from file: /data/metadata/om/certs/4.crt.

moveManager = new MoveManager(replicationManager, containerManager);
containerReplicaPendingOps.registerSubscriber(moveManager);
containerBalancer = new ContainerBalancer(this);
LOG.info(containerBalancer.toString());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Old:

2024-02-01 17:54:28,451 [main] INFO server.StorageContainerManager: 
Container Balancer status:
Key                            Value
Running                        false
Container Balancer Configuration values:
Key                                                Value
Threshold                                          10
Max Datanodes to Involve per Iteration(percent)    20
Max Size to Move per Iteration                     500GB
Max Size Entering Target per Iteration             26GB
Max Size Leaving Source per Iteration              26GB
Number of Iterations                               10
Time Limit for Single Container's Movement         65min
Time Limit for Single Container's Replication      50min
Interval between each Iteration                    70min
Whether to Enable Network Topology                 false
Whether to Trigger Refresh Datanode Usage Info     false
Container IDs to Exclude from Balancing            None
Datanodes Specified to be Balanced                 None
Datanodes Excluded from Balancing                  None

@adoroszlai adoroszlai merged commit ddf53ca into apache:master Feb 2, 2024
@adoroszlai
Copy link
Copy Markdown
Contributor

Thanks @nandakumar131 for the patch, @errose28, @Galsza for the review.

@nandakumar131
Copy link
Copy Markdown
Contributor Author

Thanks @adoroszlai for capturing and adding the logs! 🙂

Next time, I will remember to add the relevant log changes for these kinds of PRs.

@nandakumar131 nandakumar131 deleted the HDDS-10271 branch February 4, 2024 14:28
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.

4 participants