Fix reading big messages in Kafka Proxy #18079
Conversation
|
🟢 |
ydb/core/raw_socket/sock_impl.h
Outdated
| if (left > Buffer.Capacity()) { | ||
| // we send only buffer capacity, cause we know for sure that it will be written to socket without error | ||
| // there was a bug when we wrote to socket in one big batch and OS closed the connection if message was bigger than 6mb and SSL was enabled | ||
| ssize_t sendRes = Socket->Send(src + offset, Buffer.Capacity()); |
There was a problem hiding this comment.
А почему мы здесь берем размер буффера?
Не окажется ли эффективней взять, например, 1Mb?
There was a problem hiding this comment.
Согласен, тоже руки не дошли. Сделаю
| Buffer.flush(); | ||
| ssize_t res = Buffer.flush(); | ||
| if (res < 0) { | ||
| ythrow yexception() << "Error during flush of the written to socket data. Error code: " << strerror(-res) << " (" << res << ")" << "."; |
There was a problem hiding this comment.
вроде точка лишняя. везде без точки в конце предложения , но можешь проверить :)
There was a problem hiding this comment.
0 это тоже ошибка (после изменений в Send надо проверить может ли возвращаться 0)
There was a problem hiding this comment.
вроде точка лишняя. везде без точки в конце предложения , но можешь проверить :)
Это все GPT) Поправлю
| void TKafkaWritable::write(const char* val, size_t length) { | ||
| Buffer.write(val, length); | ||
| ssize_t res = Buffer.write(val, length); | ||
| if (res < 0) { |
There was a problem hiding this comment.
0 это тоже ошибка (после изменений в Send надо проверить может ли возвращаться 0)
There was a problem hiding this comment.
Если 0 считать ошибкой, падают все тесты. Оставлю пока как есть и заведу тикет, чтобы разобраться в этом позже.
ydb/core/raw_socket/sock_impl.h
Outdated
|
|
||
| private: | ||
| const ui32 MAX_RETRY_ATTEMPTS = 3; | ||
| const size_t MAX_SOCKET_BATCH_SIZE = 1 * 1024 * 1024; // 1 mb |
There was a problem hiding this comment.
1_MB можно использовать вместо 1 * 1024 * 1024
There was a problem hiding this comment.
и лучше static constexpr
Co-authored-by: Nikolay Shestakov <n.shestakov@gmail.com>
…github.com/a-serebryanskiy/ydb into ydb-topics-kafka-proxy-big-messages-hotfix
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | 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
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | 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 |
Co-authored-by: Nikolay Shestakov <n.shestakov@gmail.com>
Co-authored-by: Nikolay Shestakov <n.shestakov@gmail.com>
Co-authored-by: Nikolay Shestakov <n.shestakov@gmail.com>
Co-authored-by: Nikolay Shestakov <n.shestakov@gmail.com>
a-serebryanskiy
changed the title
…ig-messages-hotfix-24-4 Fix reading big messages in Kafka Proxy (from #18079)
…ig-messages-hotfix-24-4-4 Fix reading big messages in Kafka Proxy (from #18079)
…ig-messages-hotfix-25-1-1 Fix reading big messages in Kafka Proxy (from #18079)
…ig-messages-hotfix-25-1 Fix reading big messages in Kafka Proxy (from #18079)
Co-authored-by: Nikolay Shestakov <n.shestakov@gmail.com>
3 participants
Changelog entry
Issue: #18116
This PR fixes optimisation in
NKikimr::NRawSocket::TBufferedWriter, that wrote the entire message directly to socket if message was larger than available space in buffer. When we wrote more than 6mb to socket with SSL enabled, it every time returned -11 (WAGAIN).As a quick fix, we replace sending of an entire message to socket with cutting this message into 1mb chunks and sending them to socket one by one.
Changelog category
Description for reviewers
...