Skip to content

Conversation

@adoroszlai
Copy link
Contributor

What changes were proposed in this pull request?

If the DN does not receive any commands, then it will not receive the new term and drop any queued commands. This change adds the term to the top-level heartbeat response and updates datanode's record of the SCM term.

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

How was this patch tested?

Checked SCM audit logs for heartbeat response, they have term:

| INFO  | SCMAudit | user=hadoop | ip=172.23.0.2 | op=SEND_HEARTBEAT {datanodeUUID=eebacd7d-1ba4-4341-8871-4e663f6fcfdb, term=2, command=[]} | ret=SUCCESS |  
| INFO  | SCMAudit | user=hadoop | ip=172.23.0.5 | op=SEND_HEARTBEAT {datanodeUUID=cac3df6f-4877-4abb-ada0-d412240fbf7b, term=2, command=[]} | ret=SUCCESS |  
| INFO  | SCMAudit | user=hadoop | ip=172.23.0.7 | op=SEND_HEARTBEAT {datanodeUUID=a3c96822-b7e6-4c91-833d-908d05f0e891, term=2, command=[]} | ret=SUCCESS |  

Added assertion in datanode-side unit test.

https://github.com/adoroszlai/hadoop-ozone/actions/runs/3716657223

@adoroszlai adoroszlai self-assigned this Dec 16, 2022
@adoroszlai adoroszlai requested a review from sodonnel December 16, 2022 21:36
@adoroszlai adoroszlai marked this pull request as ready for review December 17, 2022 06:43
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

@sodonnel sodonnel merged commit d5f9172 into apache:master Dec 19, 2022
@adoroszlai adoroszlai deleted the HDDS-7621 branch December 19, 2022 20:44
errose28 added a commit to errose28/ozone that referenced this pull request Dec 21, 2022
* master: (88 commits)
  HDDS-7463. SCM Pipeline scrubber never able to cleanup allocated pipeline. (apache#4093)
  HDDS-7683. EC: ReplicationManager - UnderRep maintenance handler should not request nodes if none needed (apache#4109)
  HDDS-7635. Update failure metrics when allocate block fails in preExecute. (apache#4086)
  HDDS-7565. FSO purge directory for old bucket can update quota for new bucket (apache#4021)
  HDDS-7654. EC: ReplicationManager - merge mis-rep queue into under replicated queue (apache#4099)
  HDDS-7621. Update SCM term in datanode from heartbeat without any commands (apache#4101)
  HDDS-7649. S3 multipart upload EC release space quota wrong for old version (apache#4095)
  HDDS-7399. Enable specifying external root ca (apache#4053)
  HDDS-7398. Tool to remove old certs from the scm db (apache#3972)
  HDDS-6650. S3MultipartUpload support update bucket usedNamespace. (apache#4081)
  HDDS-7605. Improve logging in Container Balancer (apache#4067)
  HDDS-7616. EC: Refactor Unhealthy Replicated Processor (apache#4063)
  HDDS-7426. Add a new acceptance test for Streaming Pipeline. (apache#4019)
  HDDS-7478. [Ozone-Streaming] NPE in when creating a file with o3fs. (apache#3949)
  HDDS-7425. Add documentation for the new Streaming Pipeline feature. (apache#3913)
  HDDS-7438. [Ozone-Streaming] Add a createStreamKey method to OzoneBucket. (apache#3914)
  HDDS-7431. [Ozone-Streaming] Disable data steam by default. (apache#3900)
  HDDS-6955. [Ozone-streaming] Add explicit stream flag in ozone shell (apache#3559)
  HDDS-6867.  [Ozone-Streaming] PutKeyHandler should not use streaming to put EC key. (apache#3516)
  HDDS-6842. [Ozone-Streaming] Reduce the number of watch requests in StreamCommitWatcher. (apache#3492)
  ...
jojochuang pushed a commit to jojochuang/ozone that referenced this pull request Feb 6, 2023
…mands (apache#4101)

(cherry picked from commit d5f9172)
Change-Id: Ia71bfbb4909e235cec2bbcfc1040faec6f259fad

datanodeProtocolServer = new SCMDatanodeProtocolServer(conf, this,
eventQueue);
eventQueue, scmContext);
Copy link
Contributor

@szetszwo szetszwo May 25, 2023

Choose a reason for hiding this comment

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

@adoroszlai , scmContext is null since it is not yet initialized. Found this problem when reviewing #4683 .

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, it is not true since initializeSystemManagers will initialize it.

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