[KIP-848] Logging improvements #4692
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
emasab
force-pushed
the
dev_kip848_logging_improvements
branch
from
eeb87cd to
60caf0d
Compare
pranavrth
requested changes
Apr 18, 2024
src/rdkafka_cgrp.c
Outdated
| @@ -1370,7 +1370,7 @@ static void rd_kafka_cgrp_rejoin(rd_kafka_cgrp_t *rkcg, const char *fmt, ...) { | |||
|
|
|||
| rd_kafka_cgrp_consumer_reset(rkcg); | |||
| rd_kafka_cgrp_set_join_state(rkcg, RD_KAFKA_CGRP_JOIN_STATE_INIT); | |||
| rd_kafka_cgrp_consumer_expedite_next_heartbeat(rkcg); | |||
| rd_kafka_cgrp_consumer_expedite_next_heartbeat(rkcg, "rejoin"); | |||
There was a problem hiding this comment.
Suggested change
| rd_kafka_cgrp_consumer_expedite_next_heartbeat(rkcg, "rejoin"); | |
| rd_kafka_cgrp_consumer_expedite_next_heartbeat(rkcg, "rejoining"); |
| case RD_KAFKA_RESP_ERR_UNKNOWN_MEMBER_ID: | ||
| rd_kafka_cgrp_consumer_expedite_next_heartbeat( | ||
| rk->rk_cgrp); | ||
| rk->rk_cgrp, "OffsetFetch error: Unknown member"); |
There was a problem hiding this comment.
Suggested change
| rk->rk_cgrp, "OffsetFetch error: Unknown member"); | |
| rk->rk_cgrp, "OffsetFetch error: Unknown member id"); |
src/rdkafka_cgrp.c
Outdated
Comment on lines
2670
to
2673
| "Assignment is the same, " | ||
| "next assignment %s", | ||
| (has_next_target_assignment_to_clear | ||
| ? "cleared" | ||
| : "not cleared")); |
There was a problem hiding this comment.
Its confusing that we are not clearing even if the assignment is same. Should explain more.
src/rdkafka_cgrp.c
Outdated
| rd_kafka_topic_partition_list_destroy( | ||
| rkcg->rkcg_next_target_assignment); | ||
| rkcg->rkcg_next_target_assignment = NULL; | ||
| } | ||
|
|
||
| rd_kafka_dbg(rkcg->rkcg_rk, CGRP, "HEARTBEAT", | ||
| "Assignment is the same, " |
There was a problem hiding this comment.
We should have something like, "Reconciling/Not Reconciling with the new assignment due to: REASON"
src/rdkafka_cgrp.c
Outdated
| if (rkcg->rkcg_consumer_flags & RD_KAFKA_CGRP_CONSUMER_F_WAIT_ACK) | ||
| rd_bool_t is_assignment_different = rd_false, | ||
| has_next_target_assignment_to_clear = | ||
| rkcg->rkcg_next_target_assignment && |
There was a problem hiding this comment.
I was checking the code again and feel that this part of the check is not required - rkcg->rkcg_next_target_assignment but let's keep it for this release.
src/rdkafka_cgrp.c
Outdated
| rd_bool_t is_assignment_different = rd_false; | ||
| if (rkcg->rkcg_consumer_flags & RD_KAFKA_CGRP_CONSUMER_F_WAIT_ACK) | ||
| rd_bool_t is_assignment_different = rd_false, | ||
| has_next_target_assignment_to_clear = |
There was a problem hiding this comment.
lets keep these two variable initializations to different lines.
| @@ -3794,7 +3813,7 @@ static void rd_kafka_cgrp_op_handle_OffsetCommit(rd_kafka_t *rk, | |||
| if (rkcg->rkcg_group_protocol == | |||
| RD_KAFKA_GROUP_PROTOCOL_CONSUMER) { | |||
| rd_kafka_cgrp_consumer_expedite_next_heartbeat( | |||
| rk->rk_cgrp); | |||
| rk->rk_cgrp, "OffsetCommit error: Unknown member"); | |||
There was a problem hiding this comment.
Suggested change
| rk->rk_cgrp, "OffsetCommit error: Unknown member"); | |
| rk->rk_cgrp, "OffsetCommit error: Unknown member id"); |
There was a problem hiding this comment.
Used the same error message as line 3833, do I change it there too?
emasab
force-pushed
the
dev_kip848_logging_improvements
branch
from
b38284a to
841b104
Compare
emasab
force-pushed
the
dev_kip848_logging_improvements
branch
from
841b104 to
798196a
Compare
emasab
added a commit
that referenced
this pull request
Apr 18, 2024
anchitj
pushed a commit
that referenced
this pull request
Jun 10, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.