HDDS-10202. OmMetadataManagerImpl creates new S3Batcher for each operation #6088
Conversation
|
@Pochatkin please review |
|
@adoroszlai LGTM. Thanks for fix my mistake. |
swamirishi
left a comment
swamirishi
left a comment
There was a problem hiding this comment.
@adoroszlai Thanks for the patch, overall the patch looks good to me. Had a minor nitpicky comment, Otherwise I am fine with the entire change.
| private final Map<String, TableCacheMetrics> tableCacheMetricsMap = | ||
| new HashMap<>(); | ||
| private SnapshotChainManager snapshotChainManager; | ||
| private final S3Batcher s3Batcher = new S3Batcher() { |
There was a problem hiding this comment.
nit: How about having this object inside a constructor keeping this in a method like initializeS3Batcher()?
There was a problem hiding this comment.
There are 4 constructors. What is the benefit of duplicating the assignment in all of them?
There was a problem hiding this comment.
Converted the anonymous class to an inner one.
swamirishi
left a comment
swamirishi
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the patch @adoroszlai
|
Thanks @Pochatkin, @swamirishi for the review.
No problem. |
…ation (apache#6088) (cherry picked from commit 73f0194)
…ation (apache#6088) (cherry picked from commit 73f0194)
3 participants
What changes were proposed in this pull request?
Avoid
OmMetadataManagerImplcreating new instance ofS3Batcherfor each S3 secret operation.https://issues.apache.org/jira/browse/HDDS-10202
How was this patch tested?
CI:
https://github.com/adoroszlai/ozone/actions/runs/7643348101