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

refactor!: remove opentsdb tcp server #3828

Merged

Conversation

shuiyisong
Copy link
Contributor

@shuiyisong shuiyisong commented Apr 28, 2024

I hereby agree to the terms of the GreptimeDB CLA.

Refer to a related PR or issue link (optional)

What's changed and what's your intention?

This pr mainly removes the OpenTSDB TCP server, for

  1. it's the only protocol on raw TCP layer, which brings a lot of feature implementation differences and difficulties compared to other server.
  2. we suspect that not many users use the TCP protocol for ingesting OpenTSDB data. Users can still use the HTTP endpoint for OpenTSDB data ingestion.

We have not yet reached a final agreement. Please leave comments below if you have opposite opinion.

Checklist

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.
  • This PR does not require documentation updates.

@shuiyisong shuiyisong requested a review from a team as a code owner April 28, 2024 09:52
@github-actions github-actions bot added the docs-not-required This change does not impact docs. label Apr 28, 2024
Copy link

codecov bot commented Apr 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.29%. Comparing base (f29aebf) to head (f3e6012).
Report is 28 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3828      +/-   ##
==========================================
- Coverage   85.77%   85.29%   -0.48%     
==========================================
  Files         952      952              
  Lines      162629   162931     +302     
==========================================
- Hits       139492   138979     -513     
- Misses      23137    23952     +815     

@waynexia waynexia changed the title refactor: remove opentsdb tcp server refactor!: remove opentsdb tcp server Apr 28, 2024
@github-actions github-actions bot added the breaking-change This pull request contains breaking changes. label Apr 28, 2024
Copy link
Collaborator

@fengjiachun fengjiachun left a comment

Choose a reason for hiding this comment

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

LGTM

@tisonkun
Copy link
Collaborator

Do we have some docs to update after this PR merged?

@shuiyisong shuiyisong added docs-required This change requires docs update. and removed docs-not-required This change does not impact docs. labels Apr 29, 2024
@shuiyisong
Copy link
Contributor Author

Do we have some docs to update after this PR merged?

Yes, I'll check the doc once this pr is merged.

@tisonkun tisonkun added this pull request to the merge queue Apr 29, 2024
@shuiyisong shuiyisong removed this pull request from the merge queue due to a manual request Apr 29, 2024
@tisonkun
Copy link
Collaborator

tisonkun commented Apr 29, 2024

We have not yet reached a final agreement. Please leave comments below if you have opposite opinion.

I just saw this. Sorry. Let's move to draft to prevent unexpected merge.

@tisonkun tisonkun marked this pull request as draft April 29, 2024 02:56
@shuiyisong
Copy link
Contributor Author

Now the opentsdb_addr is removed from start command in both frontend and standalone. Related configs and docs are modified. PTAL @tisonkun

@shuiyisong shuiyisong marked this pull request as ready for review May 6, 2024 03:05
@shuiyisong shuiyisong added this pull request to the merge queue May 6, 2024
Merged via the queue into GreptimeTeam:main with commit 6e9e8fa May 6, 2024
55 checks passed
@shuiyisong shuiyisong deleted the refactor/remove_opentsdb_server branch May 6, 2024 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change This pull request contains breaking changes. docs-required This change requires docs update.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants