[Impala-ASF-CR] IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions()
Tianyi Wang has abandoned this change. ( http://gerrit.cloudera.org:8080/11641 ) Change subject: IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions() .. Abandoned -- To view, visit http://gerrit.cloudera.org:8080/11641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I0ec120f9df64d6e7e7d4978b5e190376721a6897 Gerrit-Change-Number: 11641 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions()
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11641 ) Change subject: IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions() .. Patch Set 1: Can this be abandoned for now? Don't think this is the correct fix. -- To view, visit http://gerrit.cloudera.org:8080/11641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ec120f9df64d6e7e7d4978b5e190376721a6897 Gerrit-Change-Number: 11641 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Mon, 19 Nov 2018 18:57:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions()
Tianyi Wang has posted comments on this change. (
http://gerrit.cloudera.org:8080/11641 )
Change subject: IMPALA-7670: Avoid getting the latest tables in
bulkAlterPartitions()
..
Patch Set 1:
> Patch Set 1:
>
> (1 comment)
>
> ya I don't think it is specific to V2 (updated the jira description).
I've been running concurrent invalidate and drop stats with this diff and
haven't reproduced it.
diff --git a/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
b/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
index e9ef929103..7058a384e2 100644
--- a/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
+++ b/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
@@ -424,6 +424,7 @@ public interface FeFsTable extends FeTable {
break;
}
}
+try { Thread.sleep(100);} catch (Exception e) {}
if (matchFound) return partition;
}
return null;
diff --git a/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
b/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
index 88c1fd5138..892be0a5a7 100644
--- a/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
+++ b/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
@@ -792,6 +792,7 @@ public class HdfsTable extends Table implements FeFsTable {
HdfsPartition partition = createPartition(msPartition.getSd(),
msPartition,
permCache);
addPartition(partition);
+try { Thread.sleep(100);} catch (Exception e) {}
// If the partition is null, its HDFS path does not exist, and it was
not added
// to this table's partition list. Skip the partition.
if (partition == null) continue;
diff --git a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
index 36725df84c..e2fcc785a4 100644
--- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
+++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
@@ -3275,6 +3275,7 @@ public class CatalogOpExecutor {
MetastoreShim.alterPartitions(msClient.getHiveClient(), dbName,
tableName,
hmsPartitionsSubList);
// Mark the corresponding HdfsPartition objects as dirty
+ try {Thread.sleep(3000);} catch (Exception e) {}
for (org.apache.hadoop.hive.metastore.api.Partition msPartition:
hmsPartitionsSubList) {
try {
--
To view, visit http://gerrit.cloudera.org:8080/11641
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0ec120f9df64d6e7e7d4978b5e190376721a6897
Gerrit-Change-Number: 11641
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tianyi Wang
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Fri, 12 Oct 2018 21:38:28 +
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions()
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11641 ) Change subject: IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions() .. Patch Set 1: (1 comment) ya I don't think it is specific to V2 (updated the jira description). http://gerrit.cloudera.org:8080/#/c/11641/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11641/1//COMMIT_MSG@11 PS1, Line 11: without locking > For example CatalogServiceCatalog.invalidateTable doesn't lock the table be Ya, that is kinda weird. Are you able to repro a race by adding some sleeps in bulkAlter... and invalidating in parallel or something like that? I'm still not super confident that this race can cause such a stack trace. -- To view, visit http://gerrit.cloudera.org:8080/11641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ec120f9df64d6e7e7d4978b5e190376721a6897 Gerrit-Change-Number: 11641 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 11 Oct 2018 23:05:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions()
Tianyi Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/11641 ) Change subject: IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions() .. Patch Set 1: > Can we add a targeted stress test to reproduce this? The JIRA also > seems to indicate this is a v2-only issue but I can't understand > how that would be the case. Are we sure it's a regression? We haven't been able to reproduce it. It only happened once on V2 and it is not necessarily a V2-only issue. If my theory is correct it shouldn't be a regression. -- To view, visit http://gerrit.cloudera.org:8080/11641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ec120f9df64d6e7e7d4978b5e190376721a6897 Gerrit-Change-Number: 11641 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 11 Oct 2018 21:14:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions()
Tianyi Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/11641 ) Change subject: IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions() .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11641/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11641/1//COMMIT_MSG@11 PS1, Line 11: without locking > Just a theory. I have an impression that the table in tablecache can be rep For example CatalogServiceCatalog.invalidateTable doesn't lock the table before removing it from catalog. If bulkAlterPartitions is called concurrently it might observe a different table without locking. -- To view, visit http://gerrit.cloudera.org:8080/11641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ec120f9df64d6e7e7d4978b5e190376721a6897 Gerrit-Change-Number: 11641 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 11 Oct 2018 21:11:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions()
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11641 ) Change subject: IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions() .. Patch Set 1: Can we add a targeted stress test to reproduce this? The JIRA also seems to indicate this is a v2-only issue but I can't understand how that would be the case. Are we sure it's a regression? -- To view, visit http://gerrit.cloudera.org:8080/11641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ec120f9df64d6e7e7d4978b5e190376721a6897 Gerrit-Change-Number: 11641 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 11 Oct 2018 20:53:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions()
Tianyi Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/11641 ) Change subject: IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions() .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11641/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11641/1//COMMIT_MSG@11 PS1, Line 11: without locking > Hmm. If that is the case, some call path is violating the locking requireme Just a theory. I have an impression that the table in tablecache can be replaced even if it's locked. Let me look at that part of the code again. -- To view, visit http://gerrit.cloudera.org:8080/11641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ec120f9df64d6e7e7d4978b5e190376721a6897 Gerrit-Change-Number: 11641 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 11 Oct 2018 17:43:54 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions()
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11641 ) Change subject: IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions() .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11641/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11641/1//COMMIT_MSG@11 PS1, Line 11: without locking > I think the table is locked but bulkAlterPartitions is fetching the table a Hmm. If that is the case, some call path is violating the locking requirements. Do you have an example of that or is this just a theory? -- To view, visit http://gerrit.cloudera.org:8080/11641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ec120f9df64d6e7e7d4978b5e190376721a6897 Gerrit-Change-Number: 11641 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 11 Oct 2018 16:45:24 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions()
Tianyi Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/11641 ) Change subject: IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions() .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11641/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11641/1//COMMIT_MSG@11 PS1, Line 11: without locking > Doesn't the caller of the bulkAlterPartitions() hold the table lock? For ex I think the table is locked but bulkAlterPartitions is fetching the table again from table cache, which might be a different table object. -- To view, visit http://gerrit.cloudera.org:8080/11641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ec120f9df64d6e7e7d4978b5e190376721a6897 Gerrit-Change-Number: 11641 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 11 Oct 2018 02:35:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions()
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/11641 )
Change subject: IMPALA-7670: Avoid getting the latest tables in
bulkAlterPartitions()
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11641/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11641/1//COMMIT_MSG@11
PS1, Line 11: without locking
Doesn't the caller of the bulkAlterPartitions() hold the table lock? For
example, in the stack mentioned in the jira, dropStats() takes a table lock
before it eventually calls bulkAlterPartitions(). With this lock, is it still
possible to have concurrent modifications of the same table?
private void dropStats(TDropStatsParams params, TDdlExecResponse resp)
throws ImpalaException {
Table table = getExistingTable(params.getTable_name().getDb_name(),
params.getTable_name().getTable_name());
Preconditions.checkNotNull(table);
if (!catalog_.tryLockTable(table)) { <=
throw new InternalException(String.format("Error dropping stats for table
%s " +
"due to lock contention", table.getFullName()));
}
--
To view, visit http://gerrit.cloudera.org:8080/11641
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0ec120f9df64d6e7e7d4978b5e190376721a6897
Gerrit-Change-Number: 11641
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Thu, 11 Oct 2018 01:15:53 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11641 ) Change subject: IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions() .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11641/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/11641/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3263 PS1, Line 3263: List hmsPartitions = Lists.newArrayList(); line too long (96 > 90) -- To view, visit http://gerrit.cloudera.org:8080/11641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ec120f9df64d6e7e7d4978b5e190376721a6897 Gerrit-Change-Number: 11641 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 10 Oct 2018 19:17:50 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions()
Tianyi Wang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11641 Change subject: IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions() .. IMPALA-7670: Avoid getting the latest tables in bulkAlterPartitions() bulkAlterPartitions() needs to mark altered partitions as dirty. It currently gets the latest table using table names and mark partitions in them as dirty without locking, which could lead to concurrent modifications of a table. This patch changes it into marking the tables it operates on dirty. Change-Id: I0ec120f9df64d6e7e7d4978b5e190376721a6897 --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java 1 file changed, 21 insertions(+), 27 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/41/11641/1 -- To view, visit http://gerrit.cloudera.org:8080/11641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I0ec120f9df64d6e7e7d4978b5e190376721a6897 Gerrit-Change-Number: 11641 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Vuk Ercegovac
