You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2020/06/15 09:11:38 UTC
[GitHub] [incubator-doris] Youngwb opened a new pull request #3869: Fix BE crash when query contains HLL column
Youngwb opened a new pull request #3869:
URL: https://github.com/apache/incubator-doris/pull/3869
According #3868, BE will core because of invalid address, This bug happens occasionally because most of the time HyperLogLog's data are nullptr, which does not cause crashes, but sometimes this value is invalid address, which causes crashes during deserialize.
Whether slice data is nullptr or invalid address is determined by the RowBlock, however the RowBlock did not use `memset` when it was initialized, so invalid data may exist.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] Youngwb commented on a change in pull request #3869: Fix BE crash when query contains HLL column
Posted by GitBox <gi...@apache.org>.
Youngwb commented on a change in pull request #3869:
URL: https://github.com/apache/incubator-doris/pull/3869#discussion_r441282304
##########
File path: be/src/olap/aggregate_func.h
##########
@@ -449,15 +449,22 @@ struct AggregateFuncTraits<OLAP_FIELD_AGGREGATION_REPLACE_IF_NOT_NULL, OLAP_FIEL
template <>
struct AggregateFuncTraits<OLAP_FIELD_AGGREGATION_HLL_UNION, OLAP_FIELD_TYPE_HLL> {
static void init(RowCursorCell* dst, const char* src, bool src_null, MemPool* mem_pool, ObjectPool* agg_pool) {
- DCHECK_EQ(src_null, false);
dst->set_not_null();
auto* src_slice = reinterpret_cast<const Slice*>(src);
auto* dst_slice = reinterpret_cast<Slice*>(dst->mutable_cell_ptr());
+ // Because of history bug, we ingest some NULL HLL data in storage,
+ // which slice data is nullptr or invalid address,
+ // we must handle this case to avoid process crash.
+ HyperLogLog* hll = nullptr;
Review comment:
I think this pr is a fix for #1908, which handles the scenario where HLL data is nullptr but does not handle dirty data. And according to it's comments, it suggested to remove these check for null HLL column later.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] kangkaisen commented on a change in pull request #3869: Fix BE crash when query contains HLL column
Posted by GitBox <gi...@apache.org>.
kangkaisen commented on a change in pull request #3869:
URL: https://github.com/apache/incubator-doris/pull/3869#discussion_r440569648
##########
File path: be/src/olap/aggregate_func.h
##########
@@ -449,15 +449,22 @@ struct AggregateFuncTraits<OLAP_FIELD_AGGREGATION_REPLACE_IF_NOT_NULL, OLAP_FIEL
template <>
struct AggregateFuncTraits<OLAP_FIELD_AGGREGATION_HLL_UNION, OLAP_FIELD_TYPE_HLL> {
static void init(RowCursorCell* dst, const char* src, bool src_null, MemPool* mem_pool, ObjectPool* agg_pool) {
- DCHECK_EQ(src_null, false);
dst->set_not_null();
auto* src_slice = reinterpret_cast<const Slice*>(src);
auto* dst_slice = reinterpret_cast<Slice*>(dst->mutable_cell_ptr());
+ // Because of history bug, we ingest some NULL HLL data in storage,
+ // which slice data is nullptr or invalid address,
+ // we must handle this case to avoid process crash.
+ HyperLogLog* hll = nullptr;
Review comment:
I don't agree simply handle null here. If we do this way, we must handle null forever, which is terrible.
At least，We should find a way to convert the invalid null HLL to empty and Remove null handle in future version.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] Youngwb closed pull request #3869: Fix BE crash when query contains HLL column
Posted by GitBox <gi...@apache.org>.
Youngwb closed pull request #3869:
URL: https://github.com/apache/incubator-doris/pull/3869
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] kangkaisen commented on a change in pull request #3869: Fix BE crash when query contains HLL column
Posted by GitBox <gi...@apache.org>.
kangkaisen commented on a change in pull request #3869:
URL: https://github.com/apache/incubator-doris/pull/3869#discussion_r442710682
##########
File path: be/src/olap/aggregate_func.h
##########
@@ -449,15 +449,22 @@ struct AggregateFuncTraits<OLAP_FIELD_AGGREGATION_REPLACE_IF_NOT_NULL, OLAP_FIEL
template <>
struct AggregateFuncTraits<OLAP_FIELD_AGGREGATION_HLL_UNION, OLAP_FIELD_TYPE_HLL> {
static void init(RowCursorCell* dst, const char* src, bool src_null, MemPool* mem_pool, ObjectPool* agg_pool) {
- DCHECK_EQ(src_null, false);
dst->set_not_null();
auto* src_slice = reinterpret_cast<const Slice*>(src);
auto* dst_slice = reinterpret_cast<Slice*>(dst->mutable_cell_ptr());
+ // Because of history bug, we ingest some NULL HLL data in storage,
+ // which slice data is nullptr or invalid address,
+ // we must handle this case to avoid process crash.
+ HyperLogLog* hll = nullptr;
Review comment:
If the data has dirty, I think we should repair it.
You could reload the table data or develop a tool to repair dirty data.
I think we shouldn't make code to be compatible with dirty data. which means we forever couldn't remove the compatible code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org