ozone
ozone copied to clipboard
HDDS-10804. Reduce unnecessary Port when AllocateBlock and GetContainerWithPipeline.
What changes were proposed in this pull request?
- The current
DatanodeDetails.Port
of Ozone has a lot of ports, which will cause greater serialization and deserialization pressure (Port#Name is of String type) and may cause high memory consumption. - When reading/writing an object, most ports are not required,
- This PR will allow the client to declare the required ports when writing a request, so that SCM does not need to return all ports to the client.
RpcClient.java
private static final List<PortName> IO_PORTS =
ImmutableList.of(PortName.PORTNAME_STANDALONE, PortName.PORTNAME_RATIS);
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10804
How was this patch tested?
Unit test. Existing test
Thanks @xichen01 for working on this.
Can SCM simply always send a limited set of ports (
STANDALONE, RATIS, RATIS_DATASTREAM
) in response toAllocateBlock
requests? I don't think client needs any other ports.It would let us avoid:
- new config
- new proto for port names
- requiring client to send port names
- changing OM requests
patch: master...adoroszlai:ozone:dce755c4f3a3e409dfc4b43bc16a48cf2a068842 CI: https://github.com/adoroszlai/ozone/actions/runs/9004107560
How was this patch tested?
Unit test. Existing test
To let existing tests cover the changes, we need to set
ozone.client.limit.port.enable=true
. Since the default setting isfalse
, they only cover old behavior. Please see failures found by run withtrue
in my fork. Some of the failures are due toRATIS_DATASTREAM
port missing from the limited set.
Good idea, I will update this. Do you think that these ports (STANDALONE
, RATIS
, RATIS_DATASTREA
) is enough for Read request (getContainerWithPipeline
)?
Do you think that these ports (
STANDALONE
,RATIS
,RATIS_DATASTREA
) is enough for Read request (getContainerWithPipeline
)?
I think so. STANDALONE
is used for read and EC write, the other two for Ratis write.
@adoroszlai I have AllocateBlock
and GetContainerWithPipeline
return only IO-related ports (STANDALONE, RATIS, RATIS_DATASTREAM)
.
Thanks @xichen01 for updating the patch. I'd like someone else to also review it.
@adoroszlai PTAL, Thanks.
@errose28 please review this for compatibility
@adoroszlai @errose28 PTAL, Thanks.
@adoroszlai @errose28, Do you have any suggestions on this PR?