Skip to content

Fixes for transactions#37398

Merged
tavplubix merged 8 commits intomasterfrom
fixes_for_transactions
May 24, 2022
Merged

Fixes for transactions#37398
tavplubix merged 8 commits intomasterfrom
fixes_for_transactions

Conversation

@tavplubix
Copy link
Copy Markdown
Member

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Fixes #37133 (trivial bug)
Fixes #37094 by handling connection loss correctly (previously it was just ignored)
Fixes a bug in removal_tid deserialization.
Adds more hardening to catch even more interesting bugs.

@robot-ch-test-poll robot-ch-test-poll added the pr-not-for-changelog This PR should not be mentioned in the changelog label May 20, 2022
@tavplubix
Copy link
Copy Markdown
Member Author

Fast test (actions) - 02117_show_create_table_system - I hate this test :)

return COMMITTED;
}

bool MergeTreeTransaction::waitStateChange(CSN expected_state_csn) const
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the argument name confusing me, but in this function, we wait until csn will be not equal to expected_state_csn?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, it means "value that we expect to see when we start waiting"


Float64 elapsedSeconds() const { return elapsed.elapsedSeconds(); }

bool waitStateChange(CSN expected_state_csn) const;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a comment.

return Tx::CommittingCSN;
}

NOEXCEPT_SCOPE;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment, I cannot guess why exceptions are not allowed here.

@tavplubix
Copy link
Copy Markdown
Member Author

PullRequestCI / IntegrationTestsTsan2 (pull_request) Failing after 28m — IntegrationTestsTsan2 - Process completed with exit code 137, cc: @Felixoid

@tavplubix tavplubix merged commit 229d354 into master May 24, 2022
@tavplubix tavplubix deleted the fixes_for_transactions branch May 24, 2022 12:28
@tavplubix tavplubix mentioned this pull request May 25, 2022
tavplubix added a commit that referenced this pull request May 26, 2022
azat added a commit to azat/ClickHouse that referenced this pull request May 30, 2022
In [1] there was only few transactions, but lots of List for /test/clickhouse/txn/log:

    $ clickhouse-local --format TSVWithNamesAndTypes --file zookeeper_log.tsv.gz -q "select * except('path|session_id|event_time|thread_id|event_date|xid') apply(x->groupUniqArray(x)), path, session_id, min(event_time), max(event_time), count() from table where has_watch and type = 'Request' group by path, session_id order by count() desc limit 1 format Vertical"
    Row 1:
    ──────
    groupUniqArray(type):             ['Request']
    groupUniqArray(query_id):         ['','62d75128-9031-48a5-87ba-aec3f0b591c6']
    groupUniqArray(address):          ['::1']
    groupUniqArray(port):             [9181]
    groupUniqArray(has_watch):        [1]
    groupUniqArray(op_num):           ['List']
    groupUniqArray(data):             ['']
    groupUniqArray(is_ephemeral):     [0]
    groupUniqArray(is_sequential):    [0]
    groupUniqArray(version):          []
    groupUniqArray(requests_size):    [0]
    groupUniqArray(request_idx):      [0]
    groupUniqArray(error):            []
    groupUniqArray(watch_type):       []
    groupUniqArray(watch_state):      []
    groupUniqArray(stat_version):     [0]
    groupUniqArray(stat_cversion):    [0]
    groupUniqArray(stat_dataLength):  [0]
    groupUniqArray(stat_numChildren): [0]
    groupUniqArray(children):         [[]]
    path:                             /test/clickhouse/txn/log
    session_id:                       1
    min(event_time):                  2022-05-27 12:54:09.025897
    max(event_time):                  2022-05-27 13:37:12.846314 <!-- last transaction was at 12:54, see server log
    count():                          3673675 <-- huge

  [1]: https://s3.amazonaws.com/clickhouse-test-reports/37593/2613149f6bf4f242bbbf2c3c8539b5176fd77286/stateless_tests__debug__actions__[1/3].html

Server log:

    $ pigz -cd clickhouse-server.log.gz | fgrep TransactionLog: | head -1
    2022.05.27 12:54:09.026852 [ 5018 ] {62d75128-9031-48a5-87ba-aec3f0b591c6} <Trace> TransactionLog: Loading 33 entries from /test/clickhouse/txn/log: csn-0000000000..csn-0000000032
    $ pigz -cd clickhouse-server.log.gz | fgrep TransactionLog: | tail -1
    2022.05.27 12:54:58.909222 [ 509 ] {} <Test> TransactionLog: Closing readonly transaction (177, 38, 41b51ff1-bcba-43bf-bcea-e97ad05f6040)
    $ pigz -cd clickhouse-server.log.gz | fgrep 62d75128-9031-48a5-87ba-aec3f0b591c6 | tail -1
    2022.05.27 12:54:09.064857 [ 5018 ] {62d75128-9031-48a5-87ba-aec3f0b591c6} <Debug> MemoryTracker: Peak memory usage (for query): 0.00 B.

Fixes: ClickHouse#37398 (cc @tavplubix)
Signed-off-by: Azat Khuzhin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-not-for-changelog This PR should not be mentioned in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Logical error: TID exists Transaction rollback aborted

3 participants