Re: Review Request 69076: SENTRY-2301: Log where sentry stands in the snapshot fetching process, periodically

[Na Li via Review Board]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69076/#review212599
---


Ship it!




Ship It!

- Na Li


On Jan. 29, 2019, 9:09 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69076/
> ---
> 
> (Updated Jan. 29, 2019, 9:09 p.m.)
> 
> 
> Review request for sentry, Haley Reeve, kalyan kumar kalvagadda, Na Li, and 
> Sergio Pena.
> 
> 
> Bugs: SENTRY-2301
> https://issues.apache.org/jira/browse/SENTRY-2301
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> When sentry is fetching snapshot from HMS, it should log periodically on 
> where it stands in the snapshot process. This will help person debugging it 
> and help him understand the progress.
> 
>  
> 
> This is important as this process could take magnitude of minutes when the 
> HMS data is huge.
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java
>  2d21411e2 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>  534bb51c1 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
>  4ff3dc91a 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
>  4baeb6725 
> 
> 
> Diff: https://reviews.apache.org/r/69076/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 69076: SENTRY-2301: Log where sentry stands in the snapshot fetching process, periodically

[Arjun Mishra via Review Board]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69076/
---

(Updated Jan. 29, 2019, 9:09 p.m.)


Review request for sentry, Haley Reeve, kalyan kumar kalvagadda, Na Li, and 
Sergio Pena.


Repository: sentry


Description
---

When sentry is fetching snapshot from HMS, it should log periodically on where 
it stands in the snapshot process. This will help person debugging it and help 
him understand the progress.

 

This is important as this process could take magnitude of minutes when the HMS 
data is huge.


Diffs (updated)
-

  
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java
 2d21411e2 
  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
 534bb51c1 
  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
 4ff3dc91a 
  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
 4baeb6725 


Diff: https://reviews.apache.org/r/69076/diff/5/

Changes: https://reviews.apache.org/r/69076/diff/4-5/


Testing
---


Thanks,

Arjun Mishra

Re: Review Request 69076: SENTRY-2301: Log where sentry stands in the snapshot fetching process, periodically

[Arjun Mishra via Review Board]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69076/
---

(Updated Jan. 29, 2019, 3:43 p.m.)


Review request for sentry, Haley Reeve, kalyan kumar kalvagadda, Na Li, and 
Sergio Pena.


Repository: sentry


Description
---

When sentry is fetching snapshot from HMS, it should log periodically on where 
it stands in the snapshot process. This will help person debugging it and help 
him understand the progress.

 

This is important as this process could take magnitude of minutes when the HMS 
data is huge.


Diffs (updated)
-

  
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java
 2d21411e2 
  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
 534bb51c1 
  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
 4ff3dc91a 
  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
 4baeb6725 


Diff: https://reviews.apache.org/r/69076/diff/4/

Changes: https://reviews.apache.org/r/69076/diff/3-4/


Testing
---


Thanks,

Arjun Mishra

Re: Review Request 69076: SENTRY-2301: Log where sentry stands in the snapshot fetching process, periodically

[Arjun Mishra via Review Board]

> On Jan. 24, 2019, 11:52 p.m., kalyan kumar kalvagadda wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
> > Lines 107 (patched)
> > 
> >
> > Can you make configurarble? It might be helpfull.
> >  What do you say?

Sure


- Arjun


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69076/#review212296
---


On Jan. 24, 2019, 6:24 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69076/
> ---
> 
> (Updated Jan. 24, 2019, 6:24 p.m.)
> 
> 
> Review request for sentry, Haley Reeve, kalyan kumar kalvagadda, Na Li, and 
> Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> When sentry is fetching snapshot from HMS, it should log periodically on 
> where it stands in the snapshot process. This will help person debugging it 
> and help him understand the progress.
> 
>  
> 
> This is important as this process could take magnitude of minutes when the 
> HMS data is huge.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>  534bb51c1 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
>  4ff3dc91a 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
>  4baeb6725 
> 
> 
> Diff: https://reviews.apache.org/r/69076/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 69076: SENTRY-2301: Log where sentry stands in the snapshot fetching process, periodically

[kalyan kumar kalvagadda via Review Board]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69076/#review212296
---




sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
Lines 107 (patched)


Can you make configurarble? It might be helpfull.
 What do you say?



sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
Lines 279 (patched)


I dont't a value is printing partition-id. Instead we should be looging 
partion name. Yes, it will be big. Information that we are trying to log will 
be any way high regardless.



sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
Lines 299-300 (patched)


Change "Fetch partition objects for" to "Fetching partitions for ". This 
comment applies to tabletask and dbtask as well.



sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
Lines 319 (patched)


Printing partitionId doesn't tell you anything. Looking at the ID you can 
not derive the parition that has null storage descriptot. You should print 
partition name.



sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
Lines 555 (patched)


This doesn't look correct because results might be still getting appended 
by the tasks.(PartitionTask/TableTask/DbTask)

You should instead print the number of objects for which paths are already 
fetched. Number of objects here include database and tables.

You can use fullSnapshot.size(). This will give the total number of 
databases and tables fetches fetched so far.

This is good enough.


- kalyan kumar kalvagadda


On Jan. 24, 2019, 6:24 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69076/
> ---
> 
> (Updated Jan. 24, 2019, 6:24 p.m.)
> 
> 
> Review request for sentry, Haley Reeve, kalyan kumar kalvagadda, Na Li, and 
> Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> When sentry is fetching snapshot from HMS, it should log periodically on 
> where it stands in the snapshot process. This will help person debugging it 
> and help him understand the progress.
> 
>  
> 
> This is important as this process could take magnitude of minutes when the 
> HMS data is huge.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>  534bb51c1 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
>  4ff3dc91a 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
>  4baeb6725 
> 
> 
> Diff: https://reviews.apache.org/r/69076/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 69076: SENTRY-2301: Log where sentry stands in the snapshot fetching process, periodically

[Arjun Mishra via Review Board]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69076/
---

(Updated Jan. 18, 2019, 6:18 p.m.)


Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.


Changes
---

New patch


Repository: sentry


Description
---

When sentry is fetching snapshot from HMS, it should log periodically on where 
it stands in the snapshot process. This will help person debugging it and help 
him understand the progress.

 

This is important as this process could take magnitude of minutes when the HMS 
data is huge.


Diffs (updated)
-

  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
 534bb51c1 
  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
 4ff3dc91a 
  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
 4baeb6725 


Diff: https://reviews.apache.org/r/69076/diff/3/

Changes: https://reviews.apache.org/r/69076/diff/2-3/


Testing
---


Thanks,

Arjun Mishra

Re: Review Request 69076: SENTRY-2301: Log where sentry stands in the snapshot fetching process, periodically

[Arjun Mishra via Review Board]

> On Oct. 19, 2018, 7:56 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
> > Lines 127 (patched)
> > 
> >
> > We already have a timer for this purpose, name 
> > "org.apache.sentry.service.thrift.FullUpdateInitializer.snapshot"
> > 
> > It is defined in SentryHMSClient
> >   private final Timer updateTimer = SentryMetrics.getInstance()
> >   .getTimer(name(FullUpdateInitializer.class, SNAPSHOT));
> >   
> > It is called in 
> > SentryHMSClient.fetchFullUpdate()
> > 
> > Do we need another timer for the same purpose?
> 
> Na Li wrote:
> The existing timer is removed to be the timer specified here.

I didn't create a new timer. I simply moved it to SentryMetrics and renamed it


- Arjun


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69076/#review209799
---


On Oct. 18, 2018, 9:29 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69076/
> ---
> 
> (Updated Oct. 18, 2018, 9:29 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> When sentry is fetching snapshot from HMS, it should log periodically on 
> where it stands in the snapshot process. This will help person debugging it 
> and help him understand the progress.
> 
>  
> 
> This is important as this process could take magnitude of minutes when the 
> HMS data is huge.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>  214d78c53 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
>  3e27d1bbe 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
>  4baeb6725 
> 
> 
> Diff: https://reviews.apache.org/r/69076/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 69076: SENTRY-2301: Log where sentry stands in the snapshot fetching process, periodically

[Arjun Mishra via Review Board]

> On Oct. 19, 2018, 7:56 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
> > Line 307 (original), 313 (patched)
> > 
> >
> > You can change it to check if part.getSd() is null or not

Lina that is fixed in SENTRY-2428


- Arjun


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69076/#review209799
---


On Oct. 18, 2018, 9:29 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69076/
> ---
> 
> (Updated Oct. 18, 2018, 9:29 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> When sentry is fetching snapshot from HMS, it should log periodically on 
> where it stands in the snapshot process. This will help person debugging it 
> and help him understand the progress.
> 
>  
> 
> This is important as this process could take magnitude of minutes when the 
> HMS data is huge.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>  214d78c53 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
>  3e27d1bbe 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
>  4baeb6725 
> 
> 
> Diff: https://reviews.apache.org/r/69076/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 69076: SENTRY-2301: Log where sentry stands in the snapshot fetching process, periodically

[Arjun Mishra via Review Board]

> On Oct. 19, 2018, 6:46 p.m., Sergio Pena wrote:
> > I did a quick look and I feel this logging will be too verbose and may slow 
> > the fetch process as there is one message per task executed. There can be 
> > thousands or even millions of tasks (one per databse, one per table, one 
> > per 100 partitions).
> > 
> > Is there another way to print a progress instead of two messages per task?

Sergio we don't do one per table. We do 1 per 100 tables. If a customer had 100 
database, each database had 100 tables (1) tables and each table had 100 
partitions (1*1000 = 1000 10 million) we would have a total of 100 + 
100 + 100 = 300 tasks. So its not that many. Also we need to know what task is 
taking how much time. I will post a snippet of log messages


- Arjun


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69076/#review209797
---


On Oct. 18, 2018, 9:29 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69076/
> ---
> 
> (Updated Oct. 18, 2018, 9:29 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> When sentry is fetching snapshot from HMS, it should log periodically on 
> where it stands in the snapshot process. This will help person debugging it 
> and help him understand the progress.
> 
>  
> 
> This is important as this process could take magnitude of minutes when the 
> HMS data is huge.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>  214d78c53 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
>  3e27d1bbe 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
>  4baeb6725 
> 
> 
> Diff: https://reviews.apache.org/r/69076/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 69076: SENTRY-2301: Log where sentry stands in the snapshot fetching process, periodically

[Na Li via Review Board]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69076/#review209799
---




sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
Lines 127 (patched)


We already have a timer for this purpose, name 
"org.apache.sentry.service.thrift.FullUpdateInitializer.snapshot"

It is defined in SentryHMSClient
  private final Timer updateTimer = SentryMetrics.getInstance()
  .getTimer(name(FullUpdateInitializer.class, SNAPSHOT));
  
It is called in 
SentryHMSClient.fetchFullUpdate()

Do we need another timer for the same purpose?



sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
Line 307 (original), 313 (patched)


You can change it to check if part.getSd() is null or not


- Na Li


On Oct. 18, 2018, 9:29 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69076/
> ---
> 
> (Updated Oct. 18, 2018, 9:29 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> When sentry is fetching snapshot from HMS, it should log periodically on 
> where it stands in the snapshot process. This will help person debugging it 
> and help him understand the progress.
> 
>  
> 
> This is important as this process could take magnitude of minutes when the 
> HMS data is huge.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>  214d78c53 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
>  3e27d1bbe 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
>  4baeb6725 
> 
> 
> Diff: https://reviews.apache.org/r/69076/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 69076: SENTRY-2301: Log where sentry stands in the snapshot fetching process, periodically

[Sergio Pena via Review Board]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69076/#review209797
---



I did a quick look and I feel this logging will be too verbose and may slow the 
fetch process as there is one message per task executed. There can be thousands 
or even millions of tasks (one per databse, one per table, one per 100 
partitions).

Is there another way to print a progress instead of two messages per task?

- Sergio Pena


On Oct. 18, 2018, 9:29 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69076/
> ---
> 
> (Updated Oct. 18, 2018, 9:29 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> When sentry is fetching snapshot from HMS, it should log periodically on 
> where it stands in the snapshot process. This will help person debugging it 
> and help him understand the progress.
> 
>  
> 
> This is important as this process could take magnitude of minutes when the 
> HMS data is huge.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>  214d78c53 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
>  3e27d1bbe 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
>  4baeb6725 
> 
> 
> Diff: https://reviews.apache.org/r/69076/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 69076: SENTRY-2301: Log where sentry stands in the snapshot fetching process, periodically

[Arjun Mishra via Review Board]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69076/
---

(Updated Oct. 18, 2018, 9:29 p.m.)


Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.


Repository: sentry


Description
---

When sentry is fetching snapshot from HMS, it should log periodically on where 
it stands in the snapshot process. This will help person debugging it and help 
him understand the progress.

 

This is important as this process could take magnitude of minutes when the HMS 
data is huge.


Diffs (updated)
-

  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
 214d78c53 
  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
 3e27d1bbe 
  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
 4baeb6725 


Diff: https://reviews.apache.org/r/69076/diff/2/

Changes: https://reviews.apache.org/r/69076/diff/1-2/


Testing
---


Thanks,

Arjun Mishra

Review Request 69076: SENTRY-2301: Log where sentry stands in the snapshot fetching process, periodically

[Arjun Mishra via Review Board]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69076/
---

Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.


Repository: sentry


Description
---

When sentry is fetching snapshot from HMS, it should log periodically on where 
it stands in the snapshot process. This will help person debugging it and help 
him understand the progress.

 

This is important as this process could take magnitude of minutes when the HMS 
data is huge.


Diffs
-

  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
 214d78c53 
  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
 3e27d1bbe 
  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
 4baeb6725 


Diff: https://reviews.apache.org/r/69076/diff/1/


Testing
---


Thanks,

Arjun Mishra