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

Add QuestDB Output Component #2815

Merged
merged 10 commits into from
Sep 13, 2024
Merged

Conversation

sklarsa
Copy link
Contributor

@sklarsa sklarsa commented Aug 28, 2024

Adds an output component for writing to QuestDB tables using ILP (Influx Line Protocol) as implemented by the official QuestDB Go Client.

@CLAassistant
Copy link

CLAassistant commented Aug 28, 2024

CLA assistant check
All committers have signed the CLA.

@emaxerrno
Copy link
Contributor

This is cool

Comment on lines 53 to 62
service.NewBoolField("tlsEnabled").
Description("Use TLS to secure the connection to the server").
Optional(),
service.NewStringField("tlsVerify").
Description("Whether to verify the server's certificate. This should only be used for testing as a "+
"last resort and never used in production as it makes the connection vulnerable to "+
"man-in-the-middle attacks. Options are 'on' or 'unsafe_off'.").
Optional().
Default("on").
LintRule(`root = if ["on","unsafe_off"].contains(this != true { ["valid options are \"on\" or \"unsafe_off\"" ] }`),
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have a service.NewTLSToggledField("tls") that has default fields supported everywhere - can we please us that?

service.NewTLSToggledField("tls"),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, how's this? Our sender uses the tlsConfig contained in the unmarshalled conf.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah this LGTM, thanks!

Copy link
Collaborator

@rockwotj rockwotj left a comment

Choose a reason for hiding this comment

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

I'm not too familiar with QuestDB, but the code LGTM. Will let other team members review as well.

}

// Apply pool-level options
// todo: is this the correct interpretation of max-in-flight?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Max in flight is how many messages that connect will call concurrently with WriteBatch. So you probably want the same amount of senders as WriteBatch can be called concurrently, I think this makes sense.

return err
})

// This will flush the sender, no need to call sender.Flush at the end of the method
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe also mention this will flush and put back in the pool?

// This will flush the sender. If the flush was successful the sender is also put back in the pool.

@Jeffail Jeffail force-pushed the add-questdb-component branch from 0862ada to 649e602 Compare September 13, 2024 09:19
@Jeffail
Copy link
Collaborator

Jeffail commented Sep 13, 2024

LGTM, thanks @sklarsa!

@Jeffail Jeffail merged commit 3b9a7e8 into redpanda-data:main Sep 13, 2024
2 of 3 checks passed
@sklarsa sklarsa deleted the add-questdb-component branch September 13, 2024 13:07
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.

5 participants