-
Notifications
You must be signed in to change notification settings - Fork 5k
fix(tmq): decode heartbeat msg error if there is a consumer #34430
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
Conversation
Summary of ChangesHello @wangmm0220, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical bug in the TMQ consumer initialization process. Previously, consumers were incorrectly using a TMQ-specific connection type, leading to errors when decoding heartbeat messages. The change ensures that consumers establish connections using the appropriate query connection type, thereby resolving the decoding issue and improving the stability of TMQ consumer operations. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request addresses a bug related to heartbeat message decoding errors when a TMQ consumer is present. The fix involves changing the connection type from CONN_TYPE__TMQ to CONN_TYPE__QUERY within the tmq_consumer_new function. This change appears to be a clean and effective solution. By aligning the TMQ consumer's connection type with that of regular query connections, it likely ensures that the server sends a consistent heartbeat response format to all connections within the same application instance, thus preventing decoding errors on the client side. The dedicated heartbeat mechanism for TMQ seems to be independent and should not be affected by this change. The fix is concise and well-targeted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request changes the connection type used when creating TMQ consumers from CONN_TYPE__TMQ to CONN_TYPE__QUERY. According to the PR title, this aims to fix a "decode heartbeat msg error if there is a consumer".
Changes:
- Modified
tmq_consumer_new()function insource/client/src/clientTmq.cto useCONN_TYPE__QUERYinstead ofCONN_TYPE__TMQwhen establishing the internal connection
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // init connection | ||
| code = taos_connect_internal(conf->ip, user, pass, NULL, NULL, conf->port, CONN_TYPE__TMQ, &pTmq->pTscObj); | ||
| code = taos_connect_internal(conf->ip, user, pass, NULL, NULL, conf->port, CONN_TYPE__QUERY, &pTmq->pTscObj); |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR title mentions "decode heartbeat msg error if there is a consumer" but the description provides no details about what decoding error occurred or why changing from CONN_TYPE__TMQ to CONN_TYPE__QUERY fixes it.
Investigation shows that both connection types use identical heartbeat handlers (clientHb.c:1166-1170) and are processed identically during serialization/deserialization (tmsg.c:256,326). The main observable difference is that TMQ consumers will now appear as "QUERY" connections instead of "TMQ" connections in monitoring views (mndProfile.c:1011).
This change may be masking a root cause rather than addressing it properly. Please provide:
- A detailed description of the heartbeat decoding error that was occurring
- The specific scenario or conditions under which the error manifests
- Why changing the connection type fixes the issue
- Whether there are any side effects of TMQ consumers being identified as QUERY connections
- Reference to the issue being fixed
| code = taos_connect_internal(conf->ip, user, pass, NULL, NULL, conf->port, CONN_TYPE__QUERY, &pTmq->pTscObj); | |
| code = taos_connect_internal(conf->ip, user, pass, NULL, NULL, conf->port, CONN_TYPE__TMQ, &pTmq->pTscObj); |
|
|
||
| // init connection | ||
| code = taos_connect_internal(conf->ip, user, pass, NULL, NULL, conf->port, CONN_TYPE__TMQ, &pTmq->pTscObj); | ||
| code = taos_connect_internal(conf->ip, user, pass, NULL, NULL, conf->port, CONN_TYPE__QUERY, &pTmq->pTscObj); |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the connection type from CONN_TYPE__TMQ to CONN_TYPE__QUERY will cause TMQ consumers to be misidentified as QUERY connections in system monitoring views. This affects observability - administrators and monitoring tools will no longer be able to distinguish TMQ consumer connections from regular query connections when viewing connection information.
Reference: mndProfile.c:1011 shows that connection types are used to label connections as "QUERY", "TMQ", or "UNKNOWN" in system views. After this change, all TMQ consumers will be labeled as "QUERY" connections.
If this change is intentional to work around an issue, it should be:
- Clearly documented in the code with a comment explaining why
- Accompanied by a plan to restore proper TMQ connection identification
- Validated that monitoring and management tools can still properly handle TMQ operations
|
|
||
| // init connection | ||
| code = taos_connect_internal(conf->ip, user, pass, NULL, NULL, conf->port, CONN_TYPE__TMQ, &pTmq->pTscObj); | ||
| code = taos_connect_internal(conf->ip, user, pass, NULL, NULL, conf->port, CONN_TYPE__QUERY, &pTmq->pTscObj); |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR description is incomplete. It does not provide:
- A description of the actual changes made
- The issue being fixed (no issue link provided)
- Why this change is necessary
- What the "decode heartbeat msg error" mentioned in the title refers to
- Any potential side effects or implications of the change
A complete PR description should help reviewers understand the context, motivation, and impact of the changes. Please update the description to include this information.
Description
Issue(s)
Checklist
Please check the items in the checklist if applicable.