Skip to content

Commit 4a6d2f0

Browse files
adoroszlaiadoroszlai
authored
HDDS-14169. Reduce copying of maps in OmKeyArgs (#9631)
1 parent 50acdb1 commit 4a6d2f0

3 files changed

Lines changed: 63 additions & 40 deletions

File tree

‎hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/BlockDataStreamOutputEntryPool.java‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.time.Clock;
2323
import java.time.ZoneOffset;
2424
import java.util.ArrayList;
25+
import java.util.HashMap;
2526
import java.util.List;
2627
import java.util.ListIterator;
2728
import java.util.Map;
@@ -55,7 +56,8 @@ public class BlockDataStreamOutputEntryPool implements KeyMetadataAware {
5556
private final OzoneClientConfig config;
5657
private int currentStreamIndex;
5758
private final OzoneManagerProtocol omClient;
58-
private final OmKeyArgs keyArgs;
59+
private final OmKeyArgs.Builder keyArgs;
60+
private final Map<String, String> metadata = new HashMap<>();
5961
private final XceiverClientFactory xceiverClientFactory;
6062
private OmMultipartCommitUploadPartInfo commitUploadPartInfo;
6163
private final long openID;
@@ -83,7 +85,7 @@ public BlockDataStreamOutputEntryPool(
8385
.setReplicationConfig(replicationConfig).setDataSize(info.getDataSize())
8486
.setIsMultipartKey(isMultipart).setMultipartUploadID(uploadID)
8587
.setMultipartUploadPartNumber(partNumber)
86-
.setSortDatanodesInPipeline(true).build();
88+
.setSortDatanodesInPipeline(true);
8789
this.openID = openID;
8890
this.excludeList = createExcludeList();
8991
this.bufferList = new ArrayList<>();
@@ -163,12 +165,12 @@ void hsyncKey(long offset) throws IOException {
163165
throw new IOException("Hsync is unsupported for multipart keys.");
164166
} else {
165167
if (keyArgs.getLocationInfoList().isEmpty()) {
166-
omClient.hsyncKey(keyArgs, openID);
168+
omClient.hsyncKey(buildKeyArgs(), openID);
167169
} else {
168170
ContainerBlockID lastBLockId = keyArgs.getLocationInfoList().get(keyArgs.getLocationInfoList().size() - 1)
169171
.getBlockID().getContainerBlockID();
170172
if (!lastUpdatedBlockId.equals(lastBLockId)) {
171-
omClient.hsyncKey(keyArgs, openID);
173+
omClient.hsyncKey(buildKeyArgs(), openID);
172174
lastUpdatedBlockId = lastBLockId;
173175
}
174176
}
@@ -235,7 +237,7 @@ private void allocateNewBlock() throws IOException {
235237
LOG.debug("Allocating block with {}", excludeList);
236238
}
237239
OmKeyLocationInfo subKeyInfo =
238-
omClient.allocateBlock(keyArgs, openID, excludeList);
240+
omClient.allocateBlock(buildKeyArgs(), openID, excludeList);
239241
addKeyLocationInfo(subKeyInfo);
240242
}
241243

@@ -251,9 +253,9 @@ void commitKey(long offset) throws IOException {
251253
// partial key of a large file.
252254
if (keyArgs.getIsMultipartKey()) {
253255
commitUploadPartInfo =
254-
omClient.commitMultipartUploadPart(keyArgs, openID);
256+
omClient.commitMultipartUploadPart(buildKeyArgs(), openID);
255257
} else {
256-
omClient.commitKey(keyArgs, openID);
258+
omClient.commitKey(buildKeyArgs(), openID);
257259
}
258260
} else {
259261
LOG.warn("Closing KeyDataStreamOutput, but key args is null");
@@ -333,6 +335,11 @@ public long getDataSize() {
333335

334336
@Override
335337
public Map<String, String> getMetadata() {
336-
return this.keyArgs.getMetadata();
338+
return metadata;
339+
}
340+
341+
private OmKeyArgs buildKeyArgs() {
342+
keyArgs.addAllMetadata(metadata);
343+
return keyArgs.build();
337344
}
338345
}

‎hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/BlockOutputStreamEntryPool.java‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.time.Clock;
2424
import java.time.ZoneOffset;
2525
import java.util.ArrayList;
26+
import java.util.HashMap;
2627
import java.util.List;
2728
import java.util.ListIterator;
2829
import java.util.Map;
@@ -72,7 +73,8 @@ public class BlockOutputStreamEntryPool implements KeyMetadataAware {
7273
*/
7374
private int currentStreamIndex;
7475
private final OzoneManagerProtocol omClient;
75-
private final OmKeyArgs keyArgs;
76+
private final OmKeyArgs.Builder keyArgs;
77+
private final Map<String, String> metadata = new HashMap<>();
7678
private final XceiverClientFactory xceiverClientFactory;
7779
/**
7880
* A {@link BufferPool} shared between all
@@ -101,8 +103,7 @@ public BlockOutputStreamEntryPool(KeyOutputStream.Builder b) {
101103
.setDataSize(info.getDataSize())
102104
.setIsMultipartKey(b.isMultipartKey())
103105
.setMultipartUploadID(b.getMultipartUploadID())
104-
.setMultipartUploadPartNumber(b.getMultipartNumber())
105-
.build();
106+
.setMultipartUploadPartNumber(b.getMultipartNumber());
106107
this.openID = b.getOpenHandler().getId();
107108
this.excludeList = createExcludeList();
108109

@@ -156,7 +157,7 @@ BlockOutputStreamEntry createStreamEntry(OmKeyLocationInfo subKeyInfo, boolean f
156157
return
157158
new BlockOutputStreamEntry.Builder()
158159
.setBlockID(subKeyInfo.getBlockID())
159-
.setKey(keyArgs.getKeyName())
160+
.setKey(getKeyName())
160161
.setXceiverClientManager(xceiverClientFactory)
161162
.setPipeline(subKeyInfo.getPipeline())
162163
.setConfig(config)
@@ -300,7 +301,7 @@ private void allocateNewBlock(boolean forRetry) throws IOException {
300301
LOG.debug("Allocating block with {}", excludeList);
301302
}
302303
OmKeyLocationInfo subKeyInfo =
303-
omClient.allocateBlock(keyArgs, openID, excludeList);
304+
omClient.allocateBlock(buildKeyArgs(), openID, excludeList);
304305
addKeyLocationInfo(subKeyInfo, forRetry);
305306
}
306307

@@ -327,9 +328,9 @@ void commitKey(long offset) throws IOException {
327328
// partial key of a large file.
328329
if (keyArgs.getIsMultipartKey()) {
329330
commitUploadPartInfo =
330-
omClient.commitMultipartUploadPart(keyArgs, openID);
331+
omClient.commitMultipartUploadPart(buildKeyArgs(), openID);
331332
} else {
332-
omClient.commitKey(keyArgs, openID);
333+
omClient.commitKey(buildKeyArgs(), openID);
333334
}
334335
} else {
335336
LOG.warn("Closing KeyOutputStream, but key args is null");
@@ -349,13 +350,13 @@ void hsyncKey(long offset) throws IOException {
349350
} else {
350351
if (keyArgs.getLocationInfoList().isEmpty()) {
351352
MetricUtil.captureLatencyNs(clientMetrics::addOMHsyncLatency,
352-
() -> omClient.hsyncKey(keyArgs, openID));
353+
() -> omClient.hsyncKey(buildKeyArgs(), openID));
353354
} else {
354355
ContainerBlockID lastBLockId = keyArgs.getLocationInfoList().get(keyArgs.getLocationInfoList().size() - 1)
355356
.getBlockID().getContainerBlockID();
356357
if (!lastUpdatedBlockId.equals(lastBLockId)) {
357358
MetricUtil.captureLatencyNs(clientMetrics::addOMHsyncLatency,
358-
() -> omClient.hsyncKey(keyArgs, openID));
359+
() -> omClient.hsyncKey(buildKeyArgs(), openID));
359360
lastUpdatedBlockId = lastBLockId;
360361
}
361362
}
@@ -431,13 +432,15 @@ boolean isEmpty() {
431432

432433
@Override
433434
public Map<String, String> getMetadata() {
434-
if (keyArgs != null) {
435-
return this.keyArgs.getMetadata();
436-
}
437-
return null;
435+
return metadata;
438436
}
439437

440438
long getDataSize() {
441439
return keyArgs.getDataSize();
442440
}
441+
442+
private OmKeyArgs buildKeyArgs() {
443+
keyArgs.addAllMetadata(metadata);
444+
return keyArgs.build();
445+
}
443446
}

‎hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyArgs.java‎

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@
1919

2020
import com.google.common.annotations.VisibleForTesting;
2121
import com.google.common.collect.ImmutableList;
22+
import com.google.common.collect.ImmutableMap;
2223
import jakarta.annotation.Nonnull;
2324
import java.util.ArrayList;
24-
import java.util.HashMap;
2525
import java.util.LinkedHashMap;
2626
import java.util.List;
2727
import java.util.Map;
@@ -36,7 +36,7 @@
3636
* Args for key. Client use this to specify key's attributes on key creation
3737
* (putKey()).
3838
*/
39-
public final class OmKeyArgs implements Auditable {
39+
public final class OmKeyArgs extends WithMetadata implements Auditable {
4040
private final String volumeName;
4141
private final String bucketName;
4242
private final String keyName;
@@ -47,14 +47,13 @@ public final class OmKeyArgs implements Auditable {
4747
private final boolean isMultipartKey;
4848
private final String multipartUploadID;
4949
private final int multipartUploadPartNumber;
50-
private final Map<String, String> metadata;
5150
private final boolean sortDatanodesInPipeline;
5251
private final ImmutableList<OzoneAcl> acls;
5352
private final boolean latestVersionLocation;
5453
private final boolean recursive;
5554
private final boolean headOp;
5655
private final boolean forceUpdateContainerCacheFromSCM;
57-
private final Map<String, String> tags;
56+
private final ImmutableMap<String, String> tags;
5857
// expectedDataGeneration, when used in key creation indicates that a
5958
// key with the same keyName should exist with the given generation.
6059
// For a key commit to succeed, the original key should still be present with the
@@ -64,6 +63,7 @@ public final class OmKeyArgs implements Auditable {
6463
private Long expectedDataGeneration = null;
6564

6665
private OmKeyArgs(Builder b) {
66+
super(b);
6767
this.volumeName = b.volumeName;
6868
this.bucketName = b.bucketName;
6969
this.keyName = b.keyName;
@@ -73,15 +73,14 @@ private OmKeyArgs(Builder b) {
7373
this.isMultipartKey = b.isMultipartKey;
7474
this.multipartUploadID = b.multipartUploadID;
7575
this.multipartUploadPartNumber = b.multipartUploadPartNumber;
76-
this.metadata = b.metadata;
7776
this.acls = b.acls.build();
7877
this.sortDatanodesInPipeline = b.sortDatanodesInPipeline;
7978
this.latestVersionLocation = b.latestVersionLocation;
8079
this.recursive = b.recursive;
8180
this.headOp = b.headOp;
8281
this.forceUpdateContainerCacheFromSCM = b.forceUpdateContainerCacheFromSCM;
8382
this.ownerName = b.ownerName;
84-
this.tags = b.tags;
83+
this.tags = b.tags.build();
8584
this.expectedDataGeneration = b.expectedDataGeneration;
8685
}
8786

@@ -129,10 +128,6 @@ public void setDataSize(long size) {
129128
dataSize = size;
130129
}
131130

132-
public Map<String, String> getMetadata() {
133-
return metadata;
134-
}
135-
136131
public void setLocationInfoList(List<OmKeyLocationInfo> locationInfoList) {
137132
this.locationInfoList = locationInfoList;
138133
}
@@ -220,7 +215,7 @@ public KeyArgs toProtobuf() {
220215
/**
221216
* Builder class of OmKeyArgs.
222217
*/
223-
public static class Builder {
218+
public static class Builder extends WithMetadata.Builder {
224219
private String volumeName;
225220
private String bucketName;
226221
private String keyName;
@@ -231,14 +226,13 @@ public static class Builder {
231226
private boolean isMultipartKey;
232227
private String multipartUploadID;
233228
private int multipartUploadPartNumber;
234-
private final Map<String, String> metadata = new HashMap<>();
235229
private boolean sortDatanodesInPipeline;
236230
private boolean latestVersionLocation;
237231
private final AclListBuilder acls;
238232
private boolean recursive;
239233
private boolean headOp;
240234
private boolean forceUpdateContainerCacheFromSCM;
241-
private final Map<String, String> tags = new HashMap<>();
235+
private final MapBuilder<String, String> tags;
242236
private Long expectedDataGeneration = null;
243237

244238
public Builder() {
@@ -247,9 +241,11 @@ public Builder() {
247241

248242
private Builder(AclListBuilder acls) {
249243
this.acls = acls;
244+
this.tags = MapBuilder.empty();
250245
}
251246

252247
public Builder(OmKeyArgs obj) {
248+
super(obj);
253249
this.volumeName = obj.volumeName;
254250
this.bucketName = obj.bucketName;
255251
this.keyName = obj.keyName;
@@ -267,8 +263,7 @@ public Builder(OmKeyArgs obj) {
267263
this.forceUpdateContainerCacheFromSCM =
268264
obj.forceUpdateContainerCacheFromSCM;
269265
this.expectedDataGeneration = obj.expectedDataGeneration;
270-
this.metadata.putAll(obj.metadata);
271-
this.tags.putAll(obj.tags);
266+
this.tags = MapBuilder.of(obj.tags);
272267
this.acls = AclListBuilder.of(obj.acls);
273268
}
274269

@@ -287,6 +282,10 @@ public Builder setKeyName(String key) {
287282
return this;
288283
}
289284

285+
public String getKeyName() {
286+
return keyName;
287+
}
288+
290289
public Builder setOwnerName(String owner) {
291290
this.ownerName = owner;
292291
return this;
@@ -297,6 +296,10 @@ public Builder setDataSize(long size) {
297296
return this;
298297
}
299298

299+
public long getDataSize() {
300+
return dataSize;
301+
}
302+
300303
public Builder setReplicationConfig(ReplicationConfig replConfig) {
301304
this.replicationConfig = replConfig;
302305
return this;
@@ -307,6 +310,10 @@ public Builder setLocationInfoList(List<OmKeyLocationInfo> locationInfos) {
307310
return this;
308311
}
309312

313+
public List<OmKeyLocationInfo> getLocationInfoList() {
314+
return locationInfoList;
315+
}
316+
310317
public Builder setAcls(List<OzoneAcl> listOfAcls) {
311318
this.acls.addAll(listOfAcls);
312319
return this;
@@ -317,6 +324,10 @@ public Builder setIsMultipartKey(boolean isMultipart) {
317324
return this;
318325
}
319326

327+
public boolean getIsMultipartKey() {
328+
return isMultipartKey;
329+
}
330+
320331
public Builder setMultipartUploadID(String uploadID) {
321332
this.multipartUploadID = uploadID;
322333
return this;
@@ -327,20 +338,22 @@ public Builder setMultipartUploadPartNumber(int multipartUploadPartNumber) {
327338
return this;
328339
}
329340

341+
@Override
330342
public Builder addMetadata(String key, String value) {
331-
this.metadata.put(key, value);
343+
super.addMetadata(key, value);
332344
return this;
333345
}
334346

347+
@Override
335348
public Builder addAllMetadata(Map<String, String> metadatamap) {
336-
this.metadata.putAll(metadatamap);
349+
super.addAllMetadata(metadatamap);
337350
return this;
338351
}
339352

340353
public Builder addAllMetadataGdpr(Map<String, String> metadatamap) {
341354
addAllMetadata(metadatamap);
342-
if (Boolean.parseBoolean(metadata.get(OzoneConsts.GDPR_FLAG))) {
343-
GDPRSymmetricKey.newDefaultInstance().acceptKeyDetails(metadata::put);
355+
if (metadatamap != null && Boolean.parseBoolean(metadatamap.get(OzoneConsts.GDPR_FLAG))) {
356+
GDPRSymmetricKey.newDefaultInstance().acceptKeyDetails(this::addMetadata);
344357
}
345358
return this;
346359
}

0 commit comments

Comments
 (0)