Skip to content
This repository has been archived by the owner on Nov 24, 2023. It is now read-only.

mydumper: pipe mydumper log into dm-worker.log #93

Merged

Conversation

kennytm
Copy link
Contributor

@kennytm kennytm commented Mar 26, 2019

What problem does this PR solve?

Fix #83.

What is changed and how it works?

Instruct mydumper to write log into /dev/stderr. Then, change dm_worker to read lines from the child process's stderr, and convert into the corresponding log.* call.

Note, unlike the description of #83, in this PR

  • mydumper-$task.log is no longer created
  • logs of all levels (INFO and DEBUG) will be written into dm-worker.log, if enabled.

Check List

Tests

  • Integration test (checked that mydumper logs exist in dm-worker.log when running make integration_test)

Code changes

Side effects

  • Increased code complexity

Related changes

@kennytm kennytm added priority/normal Minor change, requires approval from ≥1 primary reviewer type/feature New feature status/PTAL This PR is ready for review. Add this label back after committing new changes labels Mar 26, 2019
@kennytm
Copy link
Contributor Author

kennytm commented Mar 27, 2019

PTAL @csuzhangxc

@IANTHEREAL
Copy link
Collaborator

Good Job! LGTM

@csuzhangxc csuzhangxc added status/LGT1 One reviewer already commented LGTM and removed status/PTAL This PR is ready for review. Add this label back after committing new changes labels Mar 27, 2019
Copy link
Member

@csuzhangxc csuzhangxc left a comment

Choose a reason for hiding this comment

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

LGTM, cool!

@csuzhangxc csuzhangxc added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/LGT1 One reviewer already commented LGTM labels Mar 27, 2019
@csuzhangxc csuzhangxc merged commit a6edf38 into pingcap:master Mar 27, 2019
@kennytm kennytm deleted the copy-mydumper-log-into-dm-worker-log branch March 27, 2019 12:16
lichunzhu pushed a commit to lichunzhu/dm that referenced this pull request Apr 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority/normal Minor change, requires approval from ≥1 primary reviewer status/LGT2 Two reviewers already commented LGTM, ready for merge type/feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants