You are viewing an old version of this page. View the current version.

Compare with Current View Page History

« Previous Version 8 Next »


1.Should Be Exception Safe

We are moving to MODERN C++ now, and Exception Safe Code will reduce the usage of LOG(FATAL) and BE will be more stable. 

1.1 Use Smart Pointer

For example, in this code, there is a raw pointer block_ptr(at line 442), and this block_ptr will pass to send_block method at line 450. It is ok in normal cases. But there is a RETURN_IF_ERROR at line 446, so it is memory leak.

If we do not use smart pointer, we could not sure that there are some unexpected return code or throw unexpected exceptions in the future. And could not prevent memory leak.


Another case, we write many code like this.

Create a new block and put it to a vector or reset it to a unique ptr or shared ptr, and the code will call delete during  free_blocks is cleared. This is unsafe, lets look at Block's constructor:

At line 63, it create many columns, if it create 10 column and 11th failed. Then exception is thrown. It will not be add to vector and not released. So that it will be memory leak.


Another case, at line 475, we push some raw pointers to a vector, but at line 474 the code may return when meet errors, so that the vector at line 471 will not release the already created ColumnPtrWrappers. 

So that not save a raw pointer in vector or other containers.

1.2 Lock and Unlock

There are many lock and unlock code. Could not make sure that the lock is correctly released. Should use unique lock or lock guard. For example, in following example, the lock is conditionally locked by checking acquire_lock

 sign. But if somebody add some code at line 76 or call return before line 86 the lock will not be correctly unlocked.

Could write code like this. Using defer lock to add lock to mutex and will be released during recvr_lock deconstructor.

2. Not Use Dynamic Arrays for Buffer

At line 62, the code create a dynamic array uint8_t meta_buff[metadata_size] as a read buffer. Dynamic array is allocated on stack and we could not make sure the size of the buffer. Sometimes it is more than 10MB. There will stack overflow.

You could use a vector or new uint8_t[metadata_size] to allocate the buffer on heap.

3. Clear EOF or EOS definitions

Currently, there are many forms that return blocks, like next_block, get_block, next_batch. And there are many ways to check if it is the last block or end of file or end of stream:

  • Depend on Status. For example, there is a END_OF_FILE status, and our storage layer code depend on this to check if it is end of file.
  • Depend on Block row num == 0;
  • Depend on nullptr, like get_block(block** res) and check if *res == nullptr;
  • Depend on an obvious eos sign, like next_block(block, bool eos)

We should unify these ways and it will make code more clear and simple. we should use this way:  next_block(block, bool eos)

But currently, if eos == true, there maybe data in block. You should check if block.rows == 0.

4. Do Not Do Heavy Work in BRPC thread

bRPC is working like Coroutine, if do some heavy work or use pthread lock, it will block the network IO.  We also discourage the usage of bhtread::mutex in our code because it is maybe a pthread that using bthread::mutex. It will

also blocking the underline IO thread in bRPC. So that we should use the following way. PUT the request into a pthread thread pool and do the work in pthread like line 225.





  • No labels