Conversation
ludv1x
force-pushed
the
CLICKHOUSE-3546
branch
3 times, most recently
from
8b1f220 to
96f3bc1
Compare
SYSTEM queries: RESTART REPLICAS SYNC REPLICA db.name STOP MERGES [db.name] START MERGES [db.name] STOP FETCHES [db.name] START FETCHES [db.name] STOP REPLICATED SENDS [db.name] START REPLICATED SENDS [db.name] STOP REPLICATION QUEUES [db.name] START REPLICATION QUEUES [db.name]
ludv1x
changed the title
dbms/src/Common/StackTrace.cpp
Outdated
| throw; | ||
| } | ||
|
|
||
| /// Dirty workaround for Asan false positive |
There was a problem hiding this comment.
- How exactly does it complain?
- This is not the only place, look at three lines above.
- Why the define is named BUILD_TYPE_Asan and not BUILD_TYPE_ASAN?
- We don't need this define because we already have ADDRESS_SANITIZER define.
There was a problem hiding this comment.
- Freeing of unallocated memory.
For some reason, it was reproduced in ClickHouse built on Ubuntu 17.10 with clang, but currently, it is not reproduced on Ubuntu 18.04.
There was a problem hiding this comment.
So, we can forgive about it.
dbms/src/Common/localBackup.h
Outdated
| * but not from a hardware failure. | ||
| */ | ||
| void localBackup(const Poco::Path & source_path, const Poco::Path & destination_path); | ||
| void localBackup(const Poco::Path & source_path, const Poco::Path & destination_path, int max_level = -1); |
There was a problem hiding this comment.
Missing comment about max_level.
Why don't use std::optional?
| InterpreterDropQuery interpreter(ast_detach, context); | ||
| interpreter.execute(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Atomicity?
There shouldn't be a moment of time when the client doesn't see that table exists.
| //// so it will not be deleted in clearOldParts. | ||
| /// If restore_covered is true, adds to the working set inactive parts, which were merged into the deleted part. | ||
| void renameAndDetachPart(const DataPartPtr & part, const String & prefix = "", bool restore_covered = false, bool move_to_detached = true); | ||
| void forgivePartAndMoveToDetached(const DataPartPtr & part, const String & prefix = "", bool restore_covered = false); |
| size_t size() const; | ||
|
|
||
| /// Do not block mutex. | ||
| void addImpl(const String & name); |
There was a problem hiding this comment.
addNonBlocking or addUnsafe?
|
|
||
| /// Returns a committed part with the given name or a part containing it. If there is no such part, returns nullptr. | ||
| DataPartPtr getActiveContainingPart(const String & part_name); | ||
| DataPartPtr getActiveContainingPartImpl(const MergeTreePartInfo & part_info, DataPartState state, DataPartsLock & lock); |
There was a problem hiding this comment.
getActiveContainingPartUnsafe or getActiveContainingPartNonBlocking or maybe just overload getActiveContainingPart?
| } | ||
|
|
||
| /// Puts into hash "stream" length of the string and its bytes | ||
| static void updateHash(SipHash & hash, const std::string & data) |
There was a problem hiding this comment.
Is it better to add SipHash::update(const std::string &) ?
There was a problem hiding this comment.
There are several implementations of string hash:
- hash(zero-terminated string)
- hash(Uint64 length, string)
- hash(VarUint64 length, string)
And it is unclear what implementation is the best to use it as the default implementation.
| *out_max_block = right; | ||
| return getFakePartNameCoveringPartRange(data.format_version, partition_id, left, right); | ||
| /// Artificial high level is choosen, to make this part "covering" all parts inside. | ||
| static constexpr UInt32 level = 999999999; |
There was a problem hiding this comment.
std::numeric_limits::max
There was a problem hiding this comment.
By some reason, this value has been already used for a long period of time.
3 participants
I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en
Also: