You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Jaume Marhuenda <ja...@gmail.com> on 2019/01/10 11:58:17 UTC
Review Request 69704: HIVE-21052
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69704/
-----------------------------------------------------------
Review request for hive.
Repository: hive-git
Description
-------
Make sure transaction get cleaned if they are aborted before addPartitions is called
Diffs
-----
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCompactor.java 2b22a62cd9
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java badcc55c91
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Cleaner.java 06b0209aa0
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java a0df82cb20
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AddDynamicPartitions.java 9c33229270
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AllocateTableWriteIdsRequest.java d05e7bac8a
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AlterPartitionsRequest.java f7d9ed2e2e
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ClearFileMetadataRequest.java f4e3d6bd71
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ClientCapabilities.java 2b394449a3
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CompactionInfoStruct.java 4aee45ce5f
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CompactionType.java 7450b27cf3
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CreationMetadata.java 9595a5dc10
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/FindSchemasByColsResp.java 42073db544
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/FireEventRequest.java dd6658d636
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetAllFunctionsResponse.java 68146e4561
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataByExprRequest.java ee535a0c80
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataByExprResult.java 71e92b6c03
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataRequest.java 0ea6ef5fb3
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataResult.java 759b495bf6
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsFilterSpec.java b5a2b68efd
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsProjectionSpec.java e6c9c06beb
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsRequest.java 7ec107ea6c
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsResponse.java faac848991
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetTablesRequest.java da361572e5
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetTablesResult.java b3cfc88b34
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/InsertEventRequestData.java f1ba64348e
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/NotificationEventRequest.java 288c365950
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/NotificationEventResponse.java b86f038c1e
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PutFileMetadataRequest.java 5cbfe64945
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/RenamePartitionRequest.java ea4cc16af5
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/SchemaVersion.java b87f65f524
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ShowCompactResponse.java 2a7b3eba2a
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 1bdbbbf363
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMFullResourcePlan.java f92e23ea20
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMGetAllResourcePlanResponse.java cd20b15be4
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMGetTriggersForResourePlanResponse.java 0fc76b9ffc
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMValidateResourcePlanResponse.java deb1569bb1
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WriteNotificationLogRequest.java 57f50b7dc8
standalone-metastore/metastore-common/src/gen/thrift/gen-php/metastore/ThriftHiveMetastore.php b94dd25b84
standalone-metastore/metastore-common/src/gen/thrift/gen-php/metastore/Types.php 2953a8f327
standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py 37db81ff73
standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ttypes.py f8b862862d
standalone-metastore/metastore-common/src/gen/thrift/gen-rb/hive_metastore_types.rb dfc5d7b294
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 748b56b0a2
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java c96bf0d90e
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java be1f8c7849
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift 7d09a5c296
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionInfo.java ea70503988
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java 8253ccb9c9
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java 2a6290315a
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java a3f5e2c754
streaming/src/java/org/apache/hive/streaming/TransactionBatch.java a625759c0b
Diff: https://reviews.apache.org/r/69704/diff/1/
Testing
-------
Planning to manually test it using the hive-spark connector, which is where this bug was discovered.
Thanks,
Jaume Marhuenda
Re: Review Request 69704: HIVE-21052
Posted by Jaume Marhuenda <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69704/
-----------------------------------------------------------
(Updated Jan. 28, 2019, 10:32 p.m.)
Review request for hive.
Repository: hive-git
Description
-------
Make sure transaction get cleaned if they are aborted before addPartitions is called
Diffs (updated)
-----
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCompactor.java dc7b2877bf
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java 5dbf634825
ql/src/java/org/apache/hadoop/hive/ql/io/HdfsUtils.java 3482cfce36
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Cleaner.java 06b0209aa0
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CleanerExecutorService.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java a0df82cb20
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/NonReentrantReadWriteLock.java PRE-CREATION
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 5e085f84af
ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestCleanerExecutorService.java PRE-CREATION
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java b6f70ebe63
shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java c569b242ae
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CompactionType.java 7450b27cf3
standalone-metastore/metastore-common/src/gen/thrift/gen-php/metastore/Types.php 13e287e352
standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ttypes.py 8f149d1d6e
standalone-metastore/metastore-common/src/gen/thrift/gen-rb/hive_metastore_types.rb 9e5f0860f2
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java be1f8c7849
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift 9576f8775a
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionInfo.java ea70503988
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java 8253ccb9c9
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java 91a9ab4053
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java 898a94dcd6
Diff: https://reviews.apache.org/r/69704/diff/4/
Changes: https://reviews.apache.org/r/69704/diff/3-4/
Testing
-------
Planning to manually test it using the hive-spark connector, which is where this bug was discovered.
Thanks,
Jaume Marhuenda
Re: Review Request 69704: HIVE-21052
Posted by Jaume Marhuenda <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69704/
-----------------------------------------------------------
(Updated Jan. 23, 2019, 9:08 p.m.)
Review request for hive.
Repository: hive-git
Description
-------
Make sure transaction get cleaned if they are aborted before addPartitions is called
Diffs (updated)
-----
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCompactor.java dc7b2877bf
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java 5dbf634825
ql/src/java/org/apache/hadoop/hive/ql/io/HdfsUtils.java 3482cfce36
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Cleaner.java 06b0209aa0
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java a0df82cb20
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 5e085f84af
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java b6f70ebe63
shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java c569b242ae
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CompactionType.java 7450b27cf3
standalone-metastore/metastore-common/src/gen/thrift/gen-php/metastore/Types.php 13e287e352
standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ttypes.py 8f149d1d6e
standalone-metastore/metastore-common/src/gen/thrift/gen-rb/hive_metastore_types.rb 9e5f0860f2
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java be1f8c7849
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift 9576f8775a
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionInfo.java ea70503988
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java 8253ccb9c9
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java 91a9ab4053
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java 898a94dcd6
Diff: https://reviews.apache.org/r/69704/diff/3/
Changes: https://reviews.apache.org/r/69704/diff/2-3/
Testing
-------
Planning to manually test it using the hive-spark connector, which is where this bug was discovered.
Thanks,
Jaume Marhuenda
Re: Review Request 69704: HIVE-21052
Posted by Eugene Koifman <ek...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69704/#review212120
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java
Line 129 (original), 129 (patched)
<https://reviews.apache.org/r/69704/#comment297738>
This doesn't check 'p' type compactions so you could enqueue multiple ones for the same table, but see my Jira comments.
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CompactionInfoStruct.java
Lines 53 (patched)
<https://reviews.apache.org/r/69704/#comment297739>
why is this needed? when is the writeId list ever get passed over the wire?
- Eugene Koifman
On Jan. 16, 2019, 10:08 a.m., Jaume Marhuenda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69704/
> -----------------------------------------------------------
>
> (Updated Jan. 16, 2019, 10:08 a.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Make sure transaction get cleaned if they are aborted before addPartitions is called
>
>
> Diffs
> -----
>
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCompactor.java dc7b2877bf
> ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java 5dbf634825
> ql/src/java/org/apache/hadoop/hive/ql/io/HdfsUtils.java 3482cfce36
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Cleaner.java 06b0209aa0
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java a0df82cb20
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 5e085f84af
> shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java b6f70ebe63
> shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java c569b242ae
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AddDynamicPartitions.java 9c33229270
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AlterPartitionsRequest.java f7d9ed2e2e
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ClearFileMetadataRequest.java f4e3d6bd71
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ClientCapabilities.java 2b394449a3
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CompactionInfoStruct.java 4aee45ce5f
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CompactionType.java 7450b27cf3
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CreationMetadata.java 9595a5dc10
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/FindSchemasByColsResp.java 42073db544
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/FireEventRequest.java dd6658d636
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetAllFunctionsResponse.java 68146e4561
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataByExprRequest.java ee535a0c80
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataByExprResult.java 71e92b6c03
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataRequest.java 0ea6ef5fb3
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataResult.java 759b495bf6
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsFilterSpec.java b5a2b68efd
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsProjectionSpec.java e6c9c06beb
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsRequest.java 7ec107ea6c
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsResponse.java faac848991
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetTablesRequest.java da361572e5
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetTablesResult.java b3cfc88b34
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/InsertEventRequestData.java f1ba64348e
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/NotificationEventRequest.java 288c365950
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/NotificationEventResponse.java b86f038c1e
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PutFileMetadataRequest.java 5cbfe64945
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/RenamePartitionRequest.java ea4cc16af5
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/SchemaVersion.java b87f65f524
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ShowCompactResponse.java 2a7b3eba2a
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 1bdbbbf363
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMFullResourcePlan.java f92e23ea20
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMGetAllResourcePlanResponse.java cd20b15be4
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMGetTriggersForResourePlanResponse.java 0fc76b9ffc
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMValidateResourcePlanResponse.java deb1569bb1
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WriteNotificationLogRequest.java 57f50b7dc8
> standalone-metastore/metastore-common/src/gen/thrift/gen-php/metastore/ThriftHiveMetastore.php b94dd25b84
> standalone-metastore/metastore-common/src/gen/thrift/gen-php/metastore/Types.php 13e287e352
> standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py 37db81ff73
> standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ttypes.py 8f149d1d6e
> standalone-metastore/metastore-common/src/gen/thrift/gen-rb/hive_metastore_types.rb 9e5f0860f2
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java be1f8c7849
> standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift 9576f8775a
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionInfo.java ea70503988
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java 8253ccb9c9
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java 91a9ab4053
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java 898a94dcd6
>
>
> Diff: https://reviews.apache.org/r/69704/diff/2/
>
>
> Testing
> -------
>
> Planning to manually test it using the hive-spark connector, which is where this bug was discovered.
>
>
> Thanks,
>
> Jaume Marhuenda
>
>
Re: Review Request 69704: HIVE-21052
Posted by Eugene Koifman <ek...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69704/#review212117
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
Lines 2539 (patched)
<https://reviews.apache.org/r/69704/#comment297735>
JavaDoc
ql/src/java/org/apache/hadoop/hive/ql/io/HdfsUtils.java
Lines 97 (patched)
<https://reviews.apache.org/r/69704/#comment297736>
JavaDoc
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Cleaner.java
Lines 83 (patched)
<https://reviews.apache.org/r/69704/#comment297728>
Since you only have a single HMS connection (I assume this is what this locks is protecting), wouldn't it be better to get the table/partition path before parallelizing the work that can actually be parallelized? This way you fork threads and then synch them immediately.
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Cleaner.java
Lines 140 (patched)
<https://reviews.apache.org/r/69704/#comment297730>
I'm not sure this achieves what the commnet says. For normal clean (as we had before) you may have > 1 compaction_queue entry in ready for cleaning. You should not have > 1 entry in Working state for the same partition, you may have > 1 entry in ready-for-cleaning since you have more workers than Cleaners.
It's perhaps made even worse by the new "table level" clean. I think you are right to worry about this though. I'll make a more detail comment on the Jira
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java
Lines 797 (patched)
<https://reviews.apache.org/r/69704/#comment297734>
Why is this needed? It should have some JavaDoc
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
Line 91 (original), 91 (patched)
<https://reviews.apache.org/r/69704/#comment297733>
I don't think this is right. You are now counting aborted txns by type, so that you need > maxAborted aborted Inserts or > maxAborted aborted Updates, etc to trigger compaction rather than ( > maxAborted of (aborted inserts + updates+ deletes)
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 1228 (original), 1231 (patched)
<https://reviews.apache.org/r/69704/#comment297737>
exclude 'p' type here
- Eugene Koifman
On Jan. 16, 2019, 10:08 a.m., Jaume Marhuenda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69704/
> -----------------------------------------------------------
>
> (Updated Jan. 16, 2019, 10:08 a.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Make sure transaction get cleaned if they are aborted before addPartitions is called
>
>
> Diffs
> -----
>
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCompactor.java dc7b2877bf
> ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java 5dbf634825
> ql/src/java/org/apache/hadoop/hive/ql/io/HdfsUtils.java 3482cfce36
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Cleaner.java 06b0209aa0
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java a0df82cb20
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 5e085f84af
> shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java b6f70ebe63
> shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java c569b242ae
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AddDynamicPartitions.java 9c33229270
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AlterPartitionsRequest.java f7d9ed2e2e
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ClearFileMetadataRequest.java f4e3d6bd71
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ClientCapabilities.java 2b394449a3
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CompactionInfoStruct.java 4aee45ce5f
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CompactionType.java 7450b27cf3
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CreationMetadata.java 9595a5dc10
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/FindSchemasByColsResp.java 42073db544
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/FireEventRequest.java dd6658d636
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetAllFunctionsResponse.java 68146e4561
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataByExprRequest.java ee535a0c80
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataByExprResult.java 71e92b6c03
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataRequest.java 0ea6ef5fb3
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataResult.java 759b495bf6
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsFilterSpec.java b5a2b68efd
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsProjectionSpec.java e6c9c06beb
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsRequest.java 7ec107ea6c
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsResponse.java faac848991
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetTablesRequest.java da361572e5
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetTablesResult.java b3cfc88b34
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/InsertEventRequestData.java f1ba64348e
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/NotificationEventRequest.java 288c365950
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/NotificationEventResponse.java b86f038c1e
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PutFileMetadataRequest.java 5cbfe64945
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/RenamePartitionRequest.java ea4cc16af5
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/SchemaVersion.java b87f65f524
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ShowCompactResponse.java 2a7b3eba2a
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 1bdbbbf363
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMFullResourcePlan.java f92e23ea20
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMGetAllResourcePlanResponse.java cd20b15be4
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMGetTriggersForResourePlanResponse.java 0fc76b9ffc
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMValidateResourcePlanResponse.java deb1569bb1
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WriteNotificationLogRequest.java 57f50b7dc8
> standalone-metastore/metastore-common/src/gen/thrift/gen-php/metastore/ThriftHiveMetastore.php b94dd25b84
> standalone-metastore/metastore-common/src/gen/thrift/gen-php/metastore/Types.php 13e287e352
> standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py 37db81ff73
> standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ttypes.py 8f149d1d6e
> standalone-metastore/metastore-common/src/gen/thrift/gen-rb/hive_metastore_types.rb 9e5f0860f2
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java be1f8c7849
> standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift 9576f8775a
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionInfo.java ea70503988
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java 8253ccb9c9
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java 91a9ab4053
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java 898a94dcd6
>
>
> Diff: https://reviews.apache.org/r/69704/diff/2/
>
>
> Testing
> -------
>
> Planning to manually test it using the hive-spark connector, which is where this bug was discovered.
>
>
> Thanks,
>
> Jaume Marhuenda
>
>
Re: Review Request 69704: HIVE-21052
Posted by Jaume Marhuenda <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69704/
-----------------------------------------------------------
(Updated Jan. 16, 2019, 6:08 p.m.)
Review request for hive.
Repository: hive-git
Description
-------
Make sure transaction get cleaned if they are aborted before addPartitions is called
Diffs (updated)
-----
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCompactor.java dc7b2877bf
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java 5dbf634825
ql/src/java/org/apache/hadoop/hive/ql/io/HdfsUtils.java 3482cfce36
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Cleaner.java 06b0209aa0
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java a0df82cb20
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 5e085f84af
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java b6f70ebe63
shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java c569b242ae
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AddDynamicPartitions.java 9c33229270
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AlterPartitionsRequest.java f7d9ed2e2e
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ClearFileMetadataRequest.java f4e3d6bd71
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ClientCapabilities.java 2b394449a3
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CompactionInfoStruct.java 4aee45ce5f
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CompactionType.java 7450b27cf3
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CreationMetadata.java 9595a5dc10
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/FindSchemasByColsResp.java 42073db544
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/FireEventRequest.java dd6658d636
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetAllFunctionsResponse.java 68146e4561
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataByExprRequest.java ee535a0c80
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataByExprResult.java 71e92b6c03
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataRequest.java 0ea6ef5fb3
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataResult.java 759b495bf6
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsFilterSpec.java b5a2b68efd
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsProjectionSpec.java e6c9c06beb
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsRequest.java 7ec107ea6c
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsResponse.java faac848991
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetTablesRequest.java da361572e5
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetTablesResult.java b3cfc88b34
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/InsertEventRequestData.java f1ba64348e
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/NotificationEventRequest.java 288c365950
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/NotificationEventResponse.java b86f038c1e
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PutFileMetadataRequest.java 5cbfe64945
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/RenamePartitionRequest.java ea4cc16af5
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/SchemaVersion.java b87f65f524
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ShowCompactResponse.java 2a7b3eba2a
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 1bdbbbf363
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMFullResourcePlan.java f92e23ea20
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMGetAllResourcePlanResponse.java cd20b15be4
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMGetTriggersForResourePlanResponse.java 0fc76b9ffc
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMValidateResourcePlanResponse.java deb1569bb1
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WriteNotificationLogRequest.java 57f50b7dc8
standalone-metastore/metastore-common/src/gen/thrift/gen-php/metastore/ThriftHiveMetastore.php b94dd25b84
standalone-metastore/metastore-common/src/gen/thrift/gen-php/metastore/Types.php 13e287e352
standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py 37db81ff73
standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ttypes.py 8f149d1d6e
standalone-metastore/metastore-common/src/gen/thrift/gen-rb/hive_metastore_types.rb 9e5f0860f2
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java be1f8c7849
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift 9576f8775a
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionInfo.java ea70503988
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java 8253ccb9c9
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java 91a9ab4053
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java 898a94dcd6
Diff: https://reviews.apache.org/r/69704/diff/2/
Changes: https://reviews.apache.org/r/69704/diff/1-2/
Testing
-------
Planning to manually test it using the hive-spark connector, which is where this bug was discovered.
Thanks,
Jaume Marhuenda