Skip to content

Conversation

@morningman
Copy link
Contributor

Proposed changes

The tablet and disk information reporting threads need to report to the FE periodically.
At the same time these two reporting threads will also be triggered by certain events.

The modification in PR #4440 caused these two threads to be triggered only by events,
and could not report regularly.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)

The tablet and disk information reporting threads need to report to the FE periodically.
At the same time these two reporting threads will also be triggered by certain events.

The modification in PR apache#4440 caused these two threads to be triggered only by events,
and could not report regularly.
@morningman morningman added the kind/fix Categorizes issue or PR as related to a bug. label Sep 14, 2020
@morningman morningman self-assigned this Sep 14, 2020
if (status != DORIS_SUCCESS) {
DorisMetrics::instance()->report_task_requests_failed->increment(1);
LOG(WARNING) << "finish report task failed. status:" << status << ", master host:"
LOG(WARNING) << "report task failed. status:" << status << ", master host:"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
LOG(WARNING) << "report task failed. status:" << status << ", master host:"
LOG(WARNING) << "report task failed. status: " << status << ", master host: "

Copy link
Member

Choose a reason for hiding this comment

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

The same to other places.

Comment on lines 983 to 985
if (submit_task) {
TAgentTaskRequest task;
listener->submit_task(task);
Copy link
Member

Choose a reason for hiding this comment

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

Seems all callers pass a 'false' parameter, how about remove these code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK


class TaskWorkerPool {
public:
// You need to modify the content in TYPE_STRING at the same time,
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it's not easy to keep order consistency some days later, how about use a function to get name of type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK

@EmmyMiao87 EmmyMiao87 added the approved Indicates a PR has been approved by one committer. label Sep 19, 2020
Copy link
Contributor

@EmmyMiao87 EmmyMiao87 left a comment

Choose a reason for hiding this comment

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

LGTM

@morningman morningman merged commit 00f25c2 into apache:master Sep 20, 2020
yiguolei pushed a commit to yiguolei/incubator-doris that referenced this pull request Sep 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. kind/fix Categorizes issue or PR as related to a bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants