YQ-3697 Add partition count to fqrun #9837
Conversation
|
⚪ |
maximyurchuk
added
the
rebase-and-check
github-actions
bot
removed
the
rebase-and-check
|
⚪
🟢 |
|
⚪ ⚪
⚪
🟢 |
|
⚪
🟢 |
|
⚪
🟢 |
| return 1; | ||
| } | ||
| fileGateway->AddDummyTopic(TDummyTopic("pq", TString(topicName), TString(filePath))); | ||
| size_t partitionCount = !partitionCountStr.empty() ? partitionCount = FromString<size_t>(partitionCountStr) : 1; |
| @@ -214,18 +215,26 @@ struct TDummyPartitionSession: public NYdb::NTopic::TPartitionSession { | |||
| std::shared_ptr<NYdb::NTopic::IReadSession> TFileTopicClient::CreateReadSession(const NYdb::NTopic::TReadSessionSettings& settings) { | |||
| Y_ENSURE(!settings.Topics_.empty()); | |||
| TString topicPath = settings.Topics_.front().Path_; | |||
There was a problem hiding this comment.
чтобы уменьшить копипасту положи settings.Topics_.front() в переменную
| Y_ENSURE(topicsIt->second.Path); | ||
|
|
||
| TString filePath; | ||
| if (TFsPath(*topicsIt->second.Path).IsDirectory()) { |
There was a problem hiding this comment.
предлагаю положить TFsPath(*topicsIt->second.Path) в отдельную переменную, у тебя этот объект строится в двух местах
| Y_ENSURE(topicsIt->second.Path); | ||
|
|
||
| TString filePath; | ||
| if (TFsPath(*topicsIt->second.Path).IsDirectory()) { |
There was a problem hiding this comment.
рекомендую *topicsIt->second.Path положить в отдельную переменную, это выражение много где встречается
| if (TFsPath(*topicsIt->second.Path).Exists() && topicsIt->second.PartitionsCount == 1) { | ||
| filePath = *topicsIt->second.Path; | ||
| } else { | ||
| filePath = TString(*topicsIt->second.Path) + "_" + ToString(partitionId); |
There was a problem hiding this comment.
тут подошел бы TStringBuilder
|
⚪
🟢 |
|
⚪
🟢 |
| struct stat stats; | ||
| if (fstat(file, &stats) != 0) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
- leaks
filefd (why not juststat? but...) - racing (between check and actual open);
... and I'm not sure, why pipe should be special-cased at all?
There was a problem hiding this comment.
желательно не использовать ifdef в нашем коде. нельзя ли заюзать какую-нибудь функцию из util | library/cpp ?
| @@ -211,21 +212,51 @@ struct TDummyPartitionSession: public NYdb::NTopic::TPartitionSession { | |||
| } | |||
| }; | |||
|
|
|||
| static bool IsPipe(const TString& filePath) { | |||
There was a problem hiding this comment.
используем анонимные неймспейсы вместо static
|
⚪ Test history | Ya make output | Test bloat
🟢 |
|
⚪ Test history | Ya make output | Test bloat
🟢 |
| if (fsPath.Exists() && topicsIt->second.PartitionsCount == 1) { | ||
| filePath = *path; | ||
| } else { | ||
| filePath = TStringBuilder() << *path << "_" << ToString(partitionId); |
|
Конфликты появились |
kardymonds
changed the title
|
🟢 |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
ydb/tests/tools/fqrun/fqrun.cpp
Outdated
| TStringBuf(others).Split(':', path, partitionCountStr); | ||
| size_t partitionCount = !partitionCountStr.empty() ? FromString<size_t>(partitionCountStr) : 1; | ||
| if (topicName.empty() || path.empty()) { | ||
| ythrow yexception() << "Incorrect table mapping, expected form topic@path[:partitions_count]" << Endl; |
There was a problem hiding this comment.
Кажется тут стоит написать Incorrect PQ file mapping или что-то в таком духе, чем Incorrect table mapping
ydb/tests/tools/fqrun/fqrun.cpp
Outdated
| } | ||
| if (!PqFilesMapping.emplace(topicName, filePath).second) { | ||
| if (!PqFilesMapping.emplace(topicName, NYql::TDummyTopic("pq", TString(topicName), TString(path), partitionCount) ).second) { |
There was a problem hiding this comment.
Лишние пробелы между скобками в ) ).second) {
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
Changelog entry
...
Changelog category
Additional information
...