You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Michael Smith (Code Review)" <ge...@cloudera.org> on 2022/07/27 21:35:27 UTC

[Impala-ASF-CR] IMPALA-11457 Fix regression with unknown disk id

Michael Smith has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18792


Change subject: IMPALA-11457 Fix regression with unknown disk id
......................................................................

IMPALA-11457 Fix regression with unknown disk id

Fixes an issue where disk ID is set to 65536 incorrectly when storage ID
is unknown due to mixed usage of short and ushort (introduced by
d6b5f82e31732c355af3f3d1a8e5da94ba9c1349). The incorrect unknown disk ID
would result in all local reads going to a single disk queue and
reducing parallelism, which specifically happens with co-located Ozone
endpoints.

This issue caused the test_admission_control_with_multiple_coords to
fail with Ozone.

Change-Id: I571ac0669ceb6a42561594c3f96723d5ed293902
---
M common/fbs/CatalogObjects.fbs
1 file changed, 2 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/92/18792/1
-- 
To view, visit http://gerrit.cloudera.org:8080/18792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I571ac0669ceb6a42561594c3f96723d5ed293902
Gerrit-Change-Number: 18792
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>

[Impala-ASF-CR] IMPALA-11457 Fix regression with unknown disk id

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18792 )

Change subject: IMPALA-11457 Fix regression with unknown disk id
......................................................................

IMPALA-11457 Fix regression with unknown disk id

Fixes an issue where disk ID is set to 65536 incorrectly when storage ID
is unknown due to mixed usage of short and ushort (introduced by
d6b5f82e31732c355af3f3d1a8e5da94ba9c1349). The incorrect unknown disk ID
would result in all local reads going to a single disk queue and
reducing parallelism, which specifically happens with co-located Ozone
endpoints.

This issue caused the test_admission_control_with_multiple_coords to
fail with Ozone.

Change-Id: I571ac0669ceb6a42561594c3f96723d5ed293902
Reviewed-on: http://gerrit.cloudera.org:8080/18792
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M common/fbs/CatalogObjects.fbs
1 file changed, 2 insertions(+), 2 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/18792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I571ac0669ceb6a42561594c3f96723d5ed293902
Gerrit-Change-Number: 18792
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-11457 Fix regression with unknown disk id

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18792 )

Change subject: IMPALA-11457 Fix regression with unknown disk id
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/11049/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/18792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I571ac0669ceb6a42561594c3f96723d5ed293902
Gerrit-Change-Number: 18792
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Jul 2022 22:02:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11457 Fix regression with unknown disk id

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18792 )

Change subject: IMPALA-11457 Fix regression with unknown disk id
......................................................................


Patch Set 1:

> Patch Set 1:
> 
> > Patch Set 1:
> > 
> > > Patch Set 1:
> > > 
> > > Nice finding! Can we reveal the bug using existing test data? Or does it only happen on Ozone?
> > 
> > I don't know of a case with HDFS where the storage ID is unknown. Maybe?
> 
> I think cloud storages like S3 will have unknown disk id as -1. But now sure if we actually use it.
> 
> If this can't be revealed by existing test data, maybe it won't worth a separated patch and we can merge it with the ozone patch.

That's fair, I can't think of any way this would show up outside of Ozone. The main implication seems to be that local reads don't get assigned individual read queues, and we only get into that scenario with Ozone.


-- 
To view, visit http://gerrit.cloudera.org:8080/18792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I571ac0669ceb6a42561594c3f96723d5ed293902
Gerrit-Change-Number: 18792
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 28 Jul 2022 16:03:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11457 Fix regression with unknown disk id

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18792 )

Change subject: IMPALA-11457 Fix regression with unknown disk id
......................................................................


Patch Set 1:

> Patch Set 1:
> 
> > Patch Set 1:
> > 
> > > Patch Set 1:
> > > 
> > > > Patch Set 1:
> > > > 
> > > > Nice finding! Can we reveal the bug using existing test data? Or does it only happen on Ozone?
> > > 
> > > I don't know of a case with HDFS where the storage ID is unknown. Maybe?
> > 
> > I think cloud storages like S3 will have unknown disk id as -1. But now sure if we actually use it.
> > 
> > If this can't be revealed by existing test data, maybe it won't worth a separated patch and we can merge it with the ozone patch.
> 
> That's fair, I can't think of any way this would show up outside of Ozone. The main implication seems to be that local reads don't get assigned individual read queues, and we only get into that scenario with Ozone.

I'm tempted to leave it separate though because Ozone is usable in Impala already, so this is an issue customers could run into. The other ticket is all internal dev/test work.


-- 
To view, visit http://gerrit.cloudera.org:8080/18792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I571ac0669ceb6a42561594c3f96723d5ed293902
Gerrit-Change-Number: 18792
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 28 Jul 2022 16:07:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11457 Fix regression with unknown disk id

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18792 )

Change subject: IMPALA-11457 Fix regression with unknown disk id
......................................................................


Patch Set 1:

Nice finding! Can we reveal the bug using existing test data? Or does it only happen on Ozone?


-- 
To view, visit http://gerrit.cloudera.org:8080/18792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I571ac0669ceb6a42561594c3f96723d5ed293902
Gerrit-Change-Number: 18792
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 28 Jul 2022 02:55:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11457 Fix regression with unknown disk id

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18792 )

Change subject: IMPALA-11457 Fix regression with unknown disk id
......................................................................


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8378/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/18792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I571ac0669ceb6a42561594c3f96723d5ed293902
Gerrit-Change-Number: 18792
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 29 Jul 2022 22:26:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11457 Fix regression with unknown disk id

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18792 )

Change subject: IMPALA-11457 Fix regression with unknown disk id
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/18792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I571ac0669ceb6a42561594c3f96723d5ed293902
Gerrit-Change-Number: 18792
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 29 Jul 2022 22:26:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11457 Fix regression with unknown disk id

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18792 )

Change subject: IMPALA-11457 Fix regression with unknown disk id
......................................................................


Patch Set 1:

> Patch Set 1:
> 
> Nice finding! Can we reveal the bug using existing test data? Or does it only happen on Ozone?

I don't know of a case with HDFS where the storage ID is unknown. Maybe?


-- 
To view, visit http://gerrit.cloudera.org:8080/18792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I571ac0669ceb6a42561594c3f96723d5ed293902
Gerrit-Change-Number: 18792
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 28 Jul 2022 02:58:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11457 Fix regression with unknown disk id

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18792 )

Change subject: IMPALA-11457 Fix regression with unknown disk id
......................................................................


Patch Set 1:

> Patch Set 1:
> 
> > Patch Set 1:
> > 
> > Nice finding! Can we reveal the bug using existing test data? Or does it only happen on Ozone?
> 
> I don't know of a case with HDFS where the storage ID is unknown. Maybe?

I think cloud storages like S3 will have unknown disk id as -1. But now sure if we actually use it.

If this can't be revealed by existing test data, maybe it won't worth a separated patch and we can merge it with the ozone patch.


-- 
To view, visit http://gerrit.cloudera.org:8080/18792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I571ac0669ceb6a42561594c3f96723d5ed293902
Gerrit-Change-Number: 18792
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 28 Jul 2022 04:02:29 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11457 Fix regression with unknown disk id

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18792 )

Change subject: IMPALA-11457 Fix regression with unknown disk id
......................................................................


Patch Set 2: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/18792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I571ac0669ceb6a42561594c3f96723d5ed293902
Gerrit-Change-Number: 18792
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 30 Jul 2022 03:21:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11457 Fix regression with unknown disk id

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18792 )

Change subject: IMPALA-11457 Fix regression with unknown disk id
......................................................................


Patch Set 1: Code-Review+2

> Patch Set 1:
> 
> > Patch Set 1:
> > 
> > > Patch Set 1:
> > > 
> > > > Patch Set 1:
> > > > 
> > > > > Patch Set 1:
> > > > > 
> > > > > Nice finding! Can we reveal the bug using existing test data? Or does it only happen on Ozone?
> > > > 
> > > > I don't know of a case with HDFS where the storage ID is unknown. Maybe?
> > > 
> > > I think cloud storages like S3 will have unknown disk id as -1. But now sure if we actually use it.
> > > 
> > > If this can't be revealed by existing test data, maybe it won't worth a separated patch and we can merge it with the ozone patch.
> > 
> > That's fair, I can't think of any way this would show up outside of Ozone. The main implication seems to be that local reads don't get assigned individual read queues, and we only get into that scenario with Ozone.
> 
> I'm tempted to leave it separate though because Ozone is usable in Impala already, so this is an issue customers could run into. The other ticket is all internal dev/test work.

Ah, that makes sense. It seems that we can't add tests due to missing Ozone in the test cluster. I'm ok to merge this without tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/18792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I571ac0669ceb6a42561594c3f96723d5ed293902
Gerrit-Change-Number: 18792
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 29 Jul 2022 01:12:29 +0000
Gerrit-HasComments: No