Skip to content

Conversation

@morningman
Copy link
Contributor

@morningman morningman commented Feb 27, 2019

  1. Extract the common part of stream load utilities from stream_load.cpp, such as StreamLoadContext, StreamLoadExecutor, move it to runtime/stream_load/

  2. Implement the routine load task executor, and can consuming data via kafka client. All implementations are in runtime/routine_load/

}

// delete TopicPartition finally
auto tp_deleter = [] (const std::vector<RdKafka::TopicPartition*>& vec) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
auto tp_deleter = [] (const std::vector<RdKafka::TopicPartition*>& vec) {
auto tp_deleter = [&topic_partitions] () {

std::for_each(vec.begin(), vec.end(),
[](RdKafka::TopicPartition* tp1) { delete tp1; });
};
DeferOp delete_tp(std::bind<void>(tp_deleter, topic_partitions));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to use std::bind

return;
}

#define HANDLE_ERROR(stmt, err_msg) \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't use macro here, you can create another function do following work, and this function call that function, and checks its return and handler error

DeferOp delete_conf(std::bind<void>(conf_deleter, conf));

std::string errstr;
#define SET_KAFKA_CONF(conf_key, conf_val) \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use lambda instead

public class RLTaskTxnCommitAttachment extends TxnCommitAttachment {

public enum RoutineLoadType {
public enum LoadSourceType {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a LoadDataSourceTypeenum in package routineload. Using 'LoadDataSourceType' is better

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants