Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add clickhouse flusher (#554) #580

Merged
merged 34 commits into from
Feb 13, 2023
Merged

Conversation

kl7sn
Copy link
Collaborator

@kl7sn kl7sn commented Dec 16, 2022

No description provided.

@CLAassistant
Copy link

CLAassistant commented Dec 16, 2022

CLA assistant check
All committers have signed the CLA.

@messixukejia messixukejia requested review from henryzhx8, yyuuttaaoo and messixukejia and removed request for henryzhx8 and yyuuttaaoo December 19, 2022 02:55
docs/cn/data-pipeline/flusher/clickhouse.md Outdated Show resolved Hide resolved
docs/cn/data-pipeline/flusher/clickhouse.md Outdated Show resolved Hide resolved
docs/cn/data-pipeline/flusher/clickhouse.md Outdated Show resolved Hide resolved
@henryzhx8 henryzhx8 self-requested a review December 20, 2022 09:16
@messixukejia
Copy link
Collaborator

上面的问题我尽快解决,最近🐑了,有点耽误进度,实在抱歉。

好好休息,保重身体,祝早日康复。

@henryzhx8 henryzhx8 self-requested a review December 21, 2022 08:34
Copy link
Collaborator

@henryzhx8 henryzhx8 left a comment

Choose a reason for hiding this comment

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

plugins/flusher/clickhouse/flusher_clickhouse.go Outdated Show resolved Hide resolved
plugins/flusher/clickhouse/flusher_clickhouse.go Outdated Show resolved Hide resolved
plugins/flusher/clickhouse/flusher_clickhouse.go Outdated Show resolved Hide resolved
@messixukejia messixukejia linked an issue Jan 14, 2023 that may be closed by this pull request
@henryzhx8 henryzhx8 self-requested a review January 16, 2023 04:48
@henryzhx8 henryzhx8 added the feature request New feature request label Jan 16, 2023
Copy link
Collaborator

@henryzhx8 henryzhx8 left a comment

Choose a reason for hiding this comment

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

  1. 必要的license加一下
  2. e2e测试没通过,需要修改一下

docs/cn/data-pipeline/flusher/clickhouse.md Outdated Show resolved Hide resolved
@kl7sn
Copy link
Collaborator Author

kl7sn commented Feb 6, 2023

我在本地执行make check-dependency-licenses,提示需要增加如下内容

image

但是该依赖是由 #603 合并到主分支产生的

image

我这边是有什么操作不对吗?执行make check-dependency-licenses的时候还有别的前置条件吗

@henryzhx8
Copy link
Collaborator

我在本地执行make check-dependency-licenses,提示需要增加如下内容

image

但是该依赖是由 #603 合并到主分支产生的

image

我这边是有什么操作不对吗?执行make check-dependency-licenses的时候还有别的前置条件吗

test目录下go mod tidy一下先

@kl7sn
Copy link
Collaborator Author

kl7sn commented Feb 7, 2023

本地运行 TEST_SCOPE=flusher_clickhouse make e2e 未出现报错。

当前报错
image

在增加 converter 之前未出现该报错,现有的 init 方法这边有什么写错的地方吗?

https://github.com/kl7sn/ilogtail/blob/flusher-clickhouse/test/engine/subscriber/clickhouse.go#L185

Copy link
Collaborator

@henryzhx8 henryzhx8 left a comment

Choose a reason for hiding this comment

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

麻烦问下你本地的go版本是多少?修改test包的时候可以重新以test目录为根目录打开编辑器,这样一些错误提示应该正常显示

test/engine/subscriber/clickhouse.go Show resolved Hide resolved
@yyuuttaaoo yyuuttaaoo added this to the v1.4 milestone Feb 13, 2023
// post them to db all at once, build statements
batch, err := f.conn.PrepareBatch(ctx, sql)
if err != nil {
return err
Copy link
Collaborator

Choose a reason for hiding this comment

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

是否需要打印error,外层是FLUSH_DATA_ALARM,但如果这里不打日志会缺少具体错误调用点

}
}
// commit and record metrics
if err = batch.Send(); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

需要return err吗

func (f *FlusherClickHouse) BufferFlush(projectName string, logstoreName string, configName string, logGroupList []*protocol.LogGroup) error {
ctx := context.Background()
sql := fmt.Sprintf(insertSQL, f.Authentication.PlainText.Database, f.Table)
for _, logGroup := range logGroupList {
Copy link
Collaborator

Choose a reason for hiding this comment

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

如果一部分flush成功一部分失败了,似乎不能处理得很好

commit 66daff1
Author: Tom Yu <yyuuttaaoo@gmail.com>
Date:   Mon Feb 13 16:49:29 2023 +0800

    Grok processor report match errors by default (alibaba#645)

    * close alibaba#644, grok report match errors on default

    meanwhile
    change pure plugin LogtailSysConfDir to current dir
    fix plugin version in logs by using ldflag to set go plugin version
@yyuuttaaoo yyuuttaaoo merged commit e8e92e0 into alibaba:main Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE]:add clickhouse flusher
6 participants