Skip to content

HDDS-5383. Eliminate expensive string creation in debug log messages - #2372

Merged
jojochuang merged 7 commits into
apache:masterfrom
sky76093016:HDDS-5383
Jun 28, 2021
Merged

HDDS-5383. Eliminate expensive string creation in debug log messages#2372
jojochuang merged 7 commits into
apache:masterfrom
sky76093016:HDDS-5383

Conversation

@sky76093016

@sky76093016sky76093016 commented Jun 28, 2021

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Eliminate expensive string creation in debug log messages(For example: toString()).

What is the link to the Apache JIRA

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

How was this patch tested?

No test.

@sky76093016

sky76093016 commented Jun 28, 2021

Copy link
Copy Markdown
ContributorAuthor

Please @jojochuang review this, thank you.

jojochuang
jojochuang requested changes Jun 28, 2021
View reviewed changes

@jojochuangjojochuang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

need to replace + with ,

Comment threadhadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockOutputStream.java Outdated
Comment threadhadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java Outdated
Comment threadhadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java Outdated
Comment threadhadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java Outdated
Comment threadhadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java Outdated
Comment threadhadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java Outdated
@sky76093016

sky76093016 commented Jun 28, 2021

Copy link
Copy Markdown
ContributorAuthor

Thanks to @jojochuang ’s review, I neglected this small problem and corrected it immediately.

jojochuang
jojochuang requested changes Jun 28, 2021
View reviewed changes
Comment threadhadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/update/server/MockCRLStore.java

if (crlInfos.isEmpty()) {
log.debug("CRL[0]: {}", crlInfos.get(0).toString());
log.debug("CRL[0]: {}", crlInfos.get(0));

@jojochuangjojochuangJun 28, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this statement doesn't make sense.
if crlInfos is empty, crlInfos.get(0) would throw IndexOutOfBoundsException

@jojochuangjojochuangJun 28, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@xiaoyuyao any idea what this was meant for?

Comment threadhadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java Outdated
@jojochuang

jojochuang commented Jun 28, 2021

Copy link
Copy Markdown
Contributor

Can you also fix these two as well?

ozone/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMSecurityProtocolFailoverProxyProvider.java

Line 213 in 2254abf

Arrays.toString(scmProxyInfoMap.values().toArray()));

ozone/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMBlockLocationFailoverProxyProvider.java

Line 189 in 2254abf

Arrays.toString(scmProxyInfoMap.values().toArray()));

given the complexity of the debug message, I suggest to wrap them with LOG.isDebugEnabled() too.

@sky76093016

sky76093016 commented Jun 28, 2021

Copy link
Copy Markdown
ContributorAuthor

Of course,I can fix these two.

jojochuang
jojochuang approved these changes Jun 28, 2021
View reviewed changes

@jojochuangjojochuang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Can you file a jira for the bug in MockCRLStore.java?

@sky76093016

sky76093016 commented Jun 28, 2021

Copy link
Copy Markdown
ContributorAuthor

Thanks again to @jojochuang ’s review, a bug about MockCRLStore.java has been posted to new jiraHDDS-5396

@jojochuang
jojochuang merged commit b7ecf8b into apache:masterJun 28, 2021
errose28 added a commit to errose28/ozone that referenced this pull request Jul 7, 2021
@errose28
Merge branch 'master' into HDDS-5337
fda5ca4
* master: (28 commits)
HDDS-5332. Add a new column family and a service provider in Recon DB for Namespace Summaries (apache#2366)
HDDS-5405. Refactor pom files for HadoopRpc and Grpc/Ratis compilation properties. (apache#2386)
HDDS-5406. add proto version to all the proto files. (apache#2385)
HDDS-5398. Avoid object creation in ReplicationManger debug log statements (apache#2379)
HDDS-5396. Fix negligence issue conditional expressions in MockCRLStore.java (apache#2380)
HDDS-5395. Avoid unnecessary numKeyOps.incr() call in OMMetrics (apache#2374)
HDDS-5389. Include ozoneserviceid in fs.defaultFS when configuring o3fs (apache#2370)
HDDS-5383. Eliminate expensive string creation in debug log messages (apache#2372)
HDDS-5380. Get more accurate space info for DedicatedDiskSpaceUsage. (apache#2365)
HDDS-5341. Container report processing is single threaded (apache#2338)
HDDS-5387. ProfileServlet to move the default output location to an ozone specific directory (apache#2368)
HDDS-5289. Update container's deleteTransactionId on creation of the transaction in SCM. (apache#2361)
HDDS-5369. Cleanup unused configuration related to SCM HA (apache#2359)
HDDS-5381. SCM terminated with exit status 1: null. (apache#2362)
HDDS-5353. Avoid unnecessary executeBatch call in insertAudits (apache#2342)
HDDS-5350 : Add allocate block support in MockOmTransport (apache#2341). Contributed by Uma Maheswara Rao G.
HDDS-4926. Support start/stop for container balancer via command line (apache#2278)
HDDS-5269. Datandoe with low ratis log volume space should not be considered for new pipeline allocation. (apache#2344)
HDDS-5367. Update modification time when updating quota/storageType/versioning (apache#2355)
HDDS-5352. java.lang.ClassNotFoundException: org/eclipse/jetty/alpn/ALPN (apache#2347)
...
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jojochuangjojochuangjojochuang approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sky76093016@jojochuang