Skip to content

HDDS-9208. Add queue limit in ReplicationServer. #5216

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ChenSammi merged 1 commit into from
Sep 13, 2023
Merged

HDDS-9208. Add queue limit in ReplicationServer. #5216

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 28 additions & 3 deletions ...ervice/src/main/java/org/apache/hadoop/ozone/container/replication/ReplicationServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,17 @@ public ReplicationServer(ContainerController controller,

int replicationServerWorkers =
replicationConfig.getReplicationMaxStreams();
LOG.info("Initializing replication server with thread count = {}",
replicationConfig.getReplicationMaxStreams());
int replicationQueueLimit =
replicationConfig.getReplicationQueueLimit();
LOG.info("Initializing replication server with thread count = {}"
+ " queue length = {}",
replicationConfig.getReplicationMaxStreams(),
replicationConfig.getReplicationQueueLimit());
this.executor =
new ThreadPoolExecutor(replicationServerWorkers,
replicationServerWorkers,
60, TimeUnit.SECONDS,
new LinkedBlockingQueue<>(),
new LinkedBlockingQueue<>(replicationQueueLimit),
new ThreadFactoryBuilder().setDaemon(true)
.setNameFormat("ReplicationContainerReader-%d")
.build());
Expand Down Expand Up @@ -153,6 +157,7 @@ public static final class ReplicationConfig {

public static final String PREFIX = "hdds.datanode.replication";
public static final String STREAMS_LIMIT_KEY = "streams.limit";
public static final String QUEUE_LIMIT = "queue.limit";

public static final String REPLICATION_STREAMS_LIMIT_KEY =
PREFIX + "." + STREAMS_LIMIT_KEY;
Expand Down Expand Up @@ -180,6 +185,18 @@ public static final class ReplicationConfig {
)
private int replicationMaxStreams = REPLICATION_MAX_STREAMS_DEFAULT;

/**
* The maximum of replication request queue length.
*/
@Config(key = QUEUE_LIMIT,
type = ConfigType.INT,
defaultValue = "4096",
Copy link
Contributor

@sumitagrawl sumitagrawl Aug 28, 2023

Choose a reason for hiding this comment

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

How this default value 4096 is derived for queue limit?

Copy link
Contributor Author

@z-bb z-bb Aug 30, 2023

Choose a reason for hiding this comment

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

@sumitagrawl, Do you think this has something to do with the failure of ci

Copy link
Contributor

@sumitagrawl sumitagrawl Aug 30, 2023

Choose a reason for hiding this comment

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

@z-bb not related to CI failure, I mean what should be appropriate default value for nettyServer handling upload/download container ... as this is given to NettyServer

Copy link
Contributor

@ChenSammi ChenSammi Sep 12, 2023

Choose a reason for hiding this comment

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

I guess 4096 this value is picked for the moment based on experience, @z-bb , right?
And Guangbao, do you remember the the biggest queue length you have ever seen before without the queue limitation imposed?

Copy link
Contributor Author

@z-bb z-bb Sep 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I guess 4096 this value is picked for the moment based on experience, @z-bb , right? And Guangbao, do you remember the the biggest queue length you have ever seen before without the queue limitation imposed?

@ChenSammi Yes, based on experience, we plan to add a monitor for the queue length. I will confirm this value later.

Copy link
Contributor

@ChenSammi ChenSammi Sep 13, 2023

Choose a reason for hiding this comment

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

@z-bb , I see. We can revisit the default value of this "queue.limit" property once you got more info from the monitor history.

tags = {DATANODE},
description = "The maximum number of queued requests for container " +
"replication"
)
private int replicationQueueLimit = 4096;

@Config(key = "port", defaultValue = "9886",
description = "Port used for the server2server replication server",
tags = {DATANODE, MANAGEMENT})
Expand Down Expand Up @@ -221,6 +238,14 @@ public void setReplicationMaxStreams(int replicationMaxStreams) {
this.replicationMaxStreams = replicationMaxStreams;
}

public int getReplicationQueueLimit() {
return replicationQueueLimit;
}

public void setReplicationQueueLimit(int limit) {
this.replicationQueueLimit = limit;
}

@PostConstruct
public void validate() {
if (replicationMaxStreams < 1) {
Expand Down