hive-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sergey Shelukhin (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (HIVE-19820) add ACID stats support to background stats updater and fix bunch of edge cases found in SU tests
Date Fri, 06 Jul 2018 21:40:00 GMT

    [ https://issues.apache.org/jira/browse/HIVE-19820?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16535380#comment-16535380
] 

Sergey Shelukhin edited comment on HIVE-19820 at 7/6/18 9:39 PM:
-----------------------------------------------------------------

Trying to write analyze pre-committing stats state makes me think it's still going to be potentially
racy, esp since most DB txns in metastore use read_committed instead of a higher concurrency
level so it's possible for some write to sneak in that invalidates the stats without updating
write ID, and analyze won't notice it.
On top of that logic is also extremely ugly and hacky and requires a new API that we then
cannot remove.

Thinking of alternative solutions (proper one would be https://issues.apache.org/jira/browse/HIVE-20109
but there's no time for that... maybe we can make sure that any path that sets the flag also
sets write ID, and any table alteration verifies that you cannot change the stats without
setting write ID (and throws otherwise, so we can catch any missing paths in tests).
We can also add back validWriteIdList in TBLS/PARTITIONS tables and only populate it on analyze
for an extra check, then clear it after any other valid stats update.

I also found additional path in set table stats/set partition stats that doesn't appear to
be updating stats state in the same DB txn as checking stats validity, when merging stats.
It may still be valid but some tests will be needed for parallel updates and reads.

This and general absence of any negative tests (parallel inserts/updates/deletes, or even
reads with someone updating stats in parallel; parallel insert+analyze, various ways to invalidate
the stats like truncate, etc. - we only have a test for a non-parallel insert without stats
collection right now for the negatives) gives me pause.
I think it would be better to push it to M05 and ideally fix HIVE-20109 (or at least as per
above make sure every stats state change sets write ID so we can detect parallel changes),
and add more tests.



was (Author: sershe):
Trying to write analyze pre-committing stats state makes me think it's still going to be potentially
racy, esp since most DB txns in metastore use read_committed instead of a higher concurrency
level so it's possible for some write to sneak in that invalidates the stats without updating
write ID, and analyze won't notice it.
On top of that logic is also extremely ugly and hacky and requires a new API that we then
cannot remove.

Thinking of alternative solutions (proper one would be https://issues.apache.org/jira/browse/HIVE-20109
but there's no time for that... maybe we can make sure that any path that sets the flag also
sets write ID, and any table alteration verifies that you cannot change the stats without
setting write ID (and throws otherwise, so we can catch any missing paths in tests).
We can also add back validWriteIdList in TBLS/PARTITIONS tables and only populate it on analyze
for an extra check, then clear it after any other valid stats update.

I also found additional path in set table stats/set partition stats that doesn't appear to
be updating stats state in the same DB txn as checking stats validity, when merging stats.
It may still be valid but some tests will be needed for parallel updates and reads.

This and general absence of any negative tests (parallel inserts/updates/deletes, or even
reads with someone updating stats in parallel; parallel insert+analyze, various ways to invalidate
the stats like truncate, etc. - we only have a test for a non-parallel insert without stats
collection right now for the negatives) gives me pause.
I think it would be better to push it to M05 and ideally fix HIVE-20109 (or at least as per
above make sure every stats state change sets write ID so we can detect parallel changes),
and add more tests.
Right now I feel this is too hot and way too under-tested; I doubt system tests that are not
specifically targeted at this will catch any subtle bugs, or even any glaring bugs e.g. for
inserts and select count running in parallel.

cc [~hagleitn] [~ekoifman] [~steveyeom2017]

> add ACID stats support to background stats updater and fix bunch of edge cases found
in SU tests
> ------------------------------------------------------------------------------------------------
>
>                 Key: HIVE-19820
>                 URL: https://issues.apache.org/jira/browse/HIVE-19820
>             Project: Hive
>          Issue Type: Sub-task
>            Reporter: Sergey Shelukhin
>            Assignee: Sergey Shelukhin
>            Priority: Major
>         Attachments: HIVE-19820.01-master-txnstats.patch, HIVE-19820.02-master-txnstats.patch,
HIVE-19820.03-master-txnstats.patch, HIVE-19820.04-master-txnstats.patch
>
>
> Follow-up from HIVE-19418.
> Right now it checks whether stats are valid in an old-fashioned way... and also gets
ACID state, and discards it without using.
> When ACID stats are implemented, ACID state needs to be used to do version-aware valid
stats checks.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Mime
View raw message