redo: split the redo writer interface to ddl writer and dml writer#4580
Conversation
ti-chi-bot
Bot
added
do-not-merge/needs-linked-issue
release-note
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR refactors the redo writer infrastructure from a unified Changes
Sequence Diagram(s)sequenceDiagram
participant Sink as Redo Sink
participant DMLWriter as DML Writer
participant DDLWriter as DDL Writer
participant Encoder as Encoding Worker
participant Storage as External Storage
Sink->>DMLWriter: AddDMLEvents(ctx, *RedoRowEvent...)
activate DMLWriter
DMLWriter->>Encoder: AddEvent(*RedoRowEvent)
activate Encoder
Encoder->>Encoder: toPolymorphicDMLEvent()
Encoder->>Encoder: Marshal & Encode
Encoder->>Storage: Write (via FileWorker)
deactivate Encoder
DMLWriter-->>Sink: error
deactivate DMLWriter
Sink->>DDLWriter: WriteDDLEvent(ctx, *DDLEvent)
activate DDLWriter
DDLWriter->>DDLWriter: toPolymorphicDDLEvent()
DDLWriter->>DDLWriter: Marshal & Encode
DDLWriter->>Storage: SyncWrite
deactivate DDLWriter
DDLWriter-->>Sink: error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello, 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 significantly refactors the redo log writing mechanism by introducing a clear separation between DDL and DML event handling. By splitting the generic 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. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a nice refactoring that splits the RedoLogWriter interface into RedoDMLWriter and RedoDDLWriter. This change adheres to the Interface Segregation Principle, making the design cleaner and the code easier to understand and maintain. The changes are applied consistently across the codebase, including the sink, writers, factory, and tests. My only concern is a minor gap in test coverage that was introduced.
|
/test all |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/redo/writer/file/file_log_writer.go (1)
95-99:⚠️ Potential issue | 🟠 MajorCheck for stopped writer before enqueueing DML events.
AppendDMLEventssends directly to the backend input channel (line 98) without checking if the writer is stopped. In contrast,WriteDDLEventchecksisStopped()first and returnsErrRedoWriterStoppedimmediately. If the backend'sencode()goroutine exits while a sender attempts to enqueue, the send will block indefinitely since the channel is never closed and the receiver is gone.Suggested fix
func (l *dmlWriter) AppendDMLEvents(ctx context.Context, events ...*commonEvent.RedoRowEvent) error { for _, event := range events { if event == nil { log.Warn("writing nil event to redo log, ignore this", zap.String("keyspace", l.cfg.ChangeFeedID().Keyspace()), zap.String("changefeed", l.cfg.ChangeFeedID().Name()), zap.String("capture", l.cfg.CaptureID())) continue } + if l.isStopped() { + return errors.ErrRedoWriterStopped.GenWithStackByArgs() + } select { case <-ctx.Done(): return ctx.Err() case l.backendWriter.GetInputCh() <- event: } } return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/redo/writer/file/file_log_writer.go` around lines 95 - 99, AppendDMLEvents currently sends to l.backendWriter.GetInputCh() without verifying the writer state, which can block if the encode goroutine exited; before attempting the send in AppendDMLEvents, call the same l.isStopped() check used by WriteDDLEvent and return ErrRedoWriterStopped if true, then proceed to the select (ctx.Done() / l.backendWriter.GetInputCh() <- event) only when not stopped so enqueueing cannot block forever when the writer is stopped.
🧹 Nitpick comments (3)
pkg/redo/writer/factory/factory_test.go (1)
35-41: Consider closing writers after test to avoid resource leaks.The test creates both
dmlWriterandddlWriterbut doesn't close them. While blackhole writers are lightweight, it's good practice to close resources in tests for consistency with real backends.Proposed fix
dmlWriter, err := NewRedoDMLWriter(context.Background(), cfg) require.NoError(t, err) require.Implements(t, (*writer.RedoDMLWriter)(nil), dmlWriter) + defer dmlWriter.Close() ddlWriter, err := NewRedoDDLWriter(context.Background(), cfg) require.NoError(t, err) require.Implements(t, (*writer.RedoDDLWriter)(nil), ddlWriter) + defer ddlWriter.Close() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/redo/writer/factory/factory_test.go` around lines 35 - 41, The test opens dmlWriter and ddlWriter via NewRedoDMLWriter and NewRedoDDLWriter but never closes them; update the test to close both writers after creation (e.g., call Close on dmlWriter and ddlWriter or defer their Close calls immediately after each successful NewRedo* call) so resources are released and the test mirrors real-backend usage.pkg/redo/writer/memory/mem_log_writer_test.go (1)
68-85: Consider adding functional tests for DML operations.
TestNewDMLWriteronly validates creation and close. Consider adding tests that exerciseAppendDMLEventsandRunto ensure the DML writer works correctly end-to-end.Would you like me to help generate a more comprehensive DML writer test that covers
AppendDMLEventsand verifies the written data?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/redo/writer/memory/mem_log_writer_test.go` around lines 68 - 85, Extend TestNewDMLWriter to exercise DML functionality by creating a DMLWriter via NewDMLWriter, calling AppendDMLEvents with a small set of synthetic DML events (insert/update/delete), invoking Run (or its background processing) to flush/commit those events, and then verifying the persisted output via the same storage URI before calling Close; target the NewDMLWriter constructor, AppendDMLEvents method, Run method and Close to ensure end-to-end behavior and include assertions that the stored data matches the appended events.pkg/redo/writer/memory/mem_log_writer.go (1)
127-129:encodeWorkers == nildoes not actually mean “writer stopped.”For instances created by
NewDMLWriter,newLogWriteralways initializesencodeWorkers, andClose()never clears it. That makes this branch a constructor-time type check rather than a lifecycle check. If the goal is to reject appends after shutdown, either rely onAddEvent’s closed-path or replace this with a real running/closed signal.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/redo/writer/memory/mem_log_writer.go` around lines 127 - 129, The check "if l.encodeWorkers == nil { return errors.ErrRedoWriterStopped... }" in mem_log_writer.go is incorrect because encodeWorkers is always initialized by newLogWriter and never cleared by Close(); replace this constructor-time check with a real shutdown signal: add a closed/running flag (e.g., an atomic uint32 or a channel) on the writer struct, set it in Close() and check it here instead (or remove this check and rely on the existing AddEvent closed-path). Update references to encodeWorkers only for worker lifecycle, and use the new flag (e.g., l.closed or l.running) in the append path to determine and return ErrRedoWriterStopped when the writer has actually been closed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/redo/writer/blackhole/blackhole_log_writer.go`:
- Around line 62-78: In AppendDMLEvents (blackHoleDMLWriter.AppendDMLEvents)
avoid dereferencing a possibly nil last element: instead find the last non-nil
event in the events slice and use its CommitTs for the log (or set current to 0
/ omit the current field if all events are nil), and only then call log.Debug;
keep the existing loop that calls e.PostFlush() for non-nil events. This
prevents a nil pointer panic when the final slice entry is nil.
---
Outside diff comments:
In `@pkg/redo/writer/file/file_log_writer.go`:
- Around line 95-99: AppendDMLEvents currently sends to
l.backendWriter.GetInputCh() without verifying the writer state, which can block
if the encode goroutine exited; before attempting the send in AppendDMLEvents,
call the same l.isStopped() check used by WriteDDLEvent and return
ErrRedoWriterStopped if true, then proceed to the select (ctx.Done() /
l.backendWriter.GetInputCh() <- event) only when not stopped so enqueueing
cannot block forever when the writer is stopped.
---
Nitpick comments:
In `@pkg/redo/writer/factory/factory_test.go`:
- Around line 35-41: The test opens dmlWriter and ddlWriter via NewRedoDMLWriter
and NewRedoDDLWriter but never closes them; update the test to close both
writers after creation (e.g., call Close on dmlWriter and ddlWriter or defer
their Close calls immediately after each successful NewRedo* call) so resources
are released and the test mirrors real-backend usage.
In `@pkg/redo/writer/memory/mem_log_writer_test.go`:
- Around line 68-85: Extend TestNewDMLWriter to exercise DML functionality by
creating a DMLWriter via NewDMLWriter, calling AppendDMLEvents with a small set
of synthetic DML events (insert/update/delete), invoking Run (or its background
processing) to flush/commit those events, and then verifying the persisted
output via the same storage URI before calling Close; target the NewDMLWriter
constructor, AppendDMLEvents method, Run method and Close to ensure end-to-end
behavior and include assertions that the stored data matches the appended
events.
In `@pkg/redo/writer/memory/mem_log_writer.go`:
- Around line 127-129: The check "if l.encodeWorkers == nil { return
errors.ErrRedoWriterStopped... }" in mem_log_writer.go is incorrect because
encodeWorkers is always initialized by newLogWriter and never cleared by
Close(); replace this constructor-time check with a real shutdown signal: add a
closed/running flag (e.g., an atomic uint32 or a channel) on the writer struct,
set it in Close() and check it here instead (or remove this check and rely on
the existing AddEvent closed-path). Update references to encodeWorkers only for
worker lifecycle, and use the new flag (e.g., l.closed or l.running) in the
append path to determine and return ErrRedoWriterStopped when the writer has
actually been closed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 97dcfc0c-b6a1-437b-a68c-af63d1c7e31a
📒 Files selected for processing (11)
downstreamadapter/sink/redo/sink.godownstreamadapter/sink/redo/sink_test.gopkg/redo/writer/blackhole/blackhole_log_writer.gopkg/redo/writer/factory/factory.gopkg/redo/writer/factory/factory_test.gopkg/redo/writer/file/file_log_writer.gopkg/redo/writer/file/file_log_writer_test.gopkg/redo/writer/memory/mem_log_writer.gopkg/redo/writer/memory/mem_log_writer_test.gopkg/redo/writer/writer.gopkg/redo/writer/writer_mock.go
|
/test all |
|
/test all |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/redo/writer/file/file_log_writer.go (1)
94-97:⚠️ Potential issue | 🟡 MinorWrap
ctx.Err()witherrors.Tracefor consistency.Line 96 returns
ctx.Err()directly, butWriteDDLEvent(line 106) wraps it witherrors.Trace(ctx.Err()). For consistency and to attach a stack trace per coding guidelines, wrap this error as well.Proposed fix
select { case <-ctx.Done(): - return ctx.Err() + return errors.Trace(ctx.Err()) case l.backendWriter.GetInputCh() <- event: }As per coding guidelines: "When an error comes from a third-party or library call in Go, wrap it immediately with
errors.Trace(err)orerrors.WrapError(...)to attach a stack trace."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/redo/writer/file/file_log_writer.go` around lines 94 - 97, The select branch that currently does "return ctx.Err()" should wrap the error with errors.Trace for consistency with WriteDDLEvent; change the return to "return errors.Trace(ctx.Err())" (ensure the errors package used elsewhere is imported) so that errors from ctx are annotated with a stack trace like other returns; locate this in the same function that calls l.backendWriter.GetInputCh() and mirrors the handling in WriteDDLEvent.
🧹 Nitpick comments (4)
pkg/redo/writer/memory/ddl_writer.go (1)
209-230: Missing error wrapping for marshal failure.On line 217, when
codec.MarshalRedoLogfails, the error is returned directly without wrapping. Per coding guidelines, errors from library calls should be wrapped witherrors.Trace(err)orerrors.WrapError(...)to attach a stack trace.♻️ Proposed fix
rawData, err := codec.MarshalRedoLog(rl, nil) if err != nil { - return nil, err + return nil, errors.WrapError(errors.ErrMarshalFailed, err) }As per coding guidelines: "When an error comes from a third-party or library call in Go, wrap it immediately with
errors.Trace(err)orerrors.WrapError(...)to attach a stack trace."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/redo/writer/memory/ddl_writer.go` around lines 209 - 230, The Marshal failure in toPolymorphicDDLEvent returns the raw error from codec.MarshalRedoLog; update the error handling in toPolymorphicDDLEvent so that the error is wrapped per project guidelines (e.g., use errors.Trace(err) or errors.WrapError(...)) before returning, referencing the codec.MarshalRedoLog call and preserving the original behavior of returning nil for the event on error.pkg/redo/writer/memory/dml_writer_test.go (1)
27-44: Consider expanding test coverage to includeRun()andAppendDMLEvents.This test only verifies that the DML writer can be constructed and closed. It doesn't exercise
Run()orAppendDMLEvents(), which are the core functionality. Consider adding tests that:
- Start the writer with
Run()in a goroutine- Append some DML events
- Verify events are processed correctly
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/redo/writer/memory/dml_writer_test.go` around lines 27 - 44, Add test coverage that exercises the DMLWriter's runtime behavior: after creating the writer with NewDMLWriter, start its Run method in a goroutine, call AppendDMLEvents with a few synthetic DML events, wait for processing (e.g., via a done channel, context cancellation, or Checkpoint/ack mechanism exposed by the writer), then assert expected side-effects and call Close; reference NewDMLWriter, Run, AppendDMLEvents, and Close to locate the code paths to exercise and synchronize on (use whatever internal signaling the DMLWriter exposes for testing if available).pkg/redo/writer/memory/ddl_writer_test.go (2)
58-62: Consider verifying the number of files written.The
WalkDircallback only asserts that each visited file has the expected name but doesn't verify how many files were written. Since 3 non-nil DDL events are written (nil is skipped), consider adding a counter to verify exactly 3 files were created.♻️ Proposed enhancement
+ fileCount := 0 err = extStorage.WalkDir(ctx, nil, func(path string, size int64) error { require.Equal(t, filename, path) + fileCount++ return nil }) require.NoError(t, err) + require.Equal(t, 3, fileCount, "expected 3 DDL log files for 3 non-nil events")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/redo/writer/memory/ddl_writer_test.go` around lines 58 - 62, The WalkDir callback currently only checks that each visited path equals filename but doesn't assert the total number of files; modify the test around extStorage.WalkDir to increment a local counter inside the callback (e.g., visitCount) for each invocation and after WalkDir returns assert require.Equal(t, 3, visitCount) (or require.NoError followed by the count assertion) to verify exactly the three non-nil DDL events were written; keep the existing path equality check in the callback.
48-56: Test exercises niltableSchemaStorepath - verify this is intentional.The test calls
WriteDDLEventwithout first callingSetTableSchemaStore, meaning the DDL events are encoded with a nil table schema store. This exercises the code path flagged inddl_writer.go. If this is intentional (the writer should handle nil gracefully), consider adding an explicit comment. Otherwise, callSetTableSchemaStorebefore writing events to test the complete workflow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/redo/writer/memory/ddl_writer_test.go` around lines 48 - 56, The test currently exercises WriteDDLEvent with a nil table schema store (the ddls loop calls lw.WriteDDLEvent without calling SetTableSchemaStore), which may be unintentional; either make the intent explicit by adding a clarifying comment above the ddls loop stating that nil tableSchemaStore behavior is intentionally tested, or update the test to call lw.SetTableSchemaStore(...) with a suitable mock/store before writing the events so the full non-nil code path in ddl_writer.go is exercised; reference the lw variable, WriteDDLEvent method, and SetTableSchemaStore to locate and apply the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/redo/writer/memory/dml_writer.go`:
- Around line 57-69: The Run method sets l.cancel which is read by Close,
causing a potential data race; update dmlWriter to initialize and set the cancel
function in a thread-safe way (e.g., add a sync.Once field or an atomic holder)
and use that mechanism in both Run and Close to ensure the cancel func is
published before Close reads it; specifically modify dmlWriter.Run to call
once.Do(func(){ l.cancel = cancel }) (or store cancel via an
atomic/atomic.Value) and modify dmlWriter.Close to retrieve and invoke the
cancel safely via the same sync.Once/atomic access so no concurrent read/write
of l.cancel occurs.
---
Outside diff comments:
In `@pkg/redo/writer/file/file_log_writer.go`:
- Around line 94-97: The select branch that currently does "return ctx.Err()"
should wrap the error with errors.Trace for consistency with WriteDDLEvent;
change the return to "return errors.Trace(ctx.Err())" (ensure the errors package
used elsewhere is imported) so that errors from ctx are annotated with a stack
trace like other returns; locate this in the same function that calls
l.backendWriter.GetInputCh() and mirrors the handling in WriteDDLEvent.
---
Nitpick comments:
In `@pkg/redo/writer/memory/ddl_writer_test.go`:
- Around line 58-62: The WalkDir callback currently only checks that each
visited path equals filename but doesn't assert the total number of files;
modify the test around extStorage.WalkDir to increment a local counter inside
the callback (e.g., visitCount) for each invocation and after WalkDir returns
assert require.Equal(t, 3, visitCount) (or require.NoError followed by the count
assertion) to verify exactly the three non-nil DDL events were written; keep the
existing path equality check in the callback.
- Around line 48-56: The test currently exercises WriteDDLEvent with a nil table
schema store (the ddls loop calls lw.WriteDDLEvent without calling
SetTableSchemaStore), which may be unintentional; either make the intent
explicit by adding a clarifying comment above the ddls loop stating that nil
tableSchemaStore behavior is intentionally tested, or update the test to call
lw.SetTableSchemaStore(...) with a suitable mock/store before writing the events
so the full non-nil code path in ddl_writer.go is exercised; reference the lw
variable, WriteDDLEvent method, and SetTableSchemaStore to locate and apply the
change.
In `@pkg/redo/writer/memory/ddl_writer.go`:
- Around line 209-230: The Marshal failure in toPolymorphicDDLEvent returns the
raw error from codec.MarshalRedoLog; update the error handling in
toPolymorphicDDLEvent so that the error is wrapped per project guidelines (e.g.,
use errors.Trace(err) or errors.WrapError(...)) before returning, referencing
the codec.MarshalRedoLog call and preserving the original behavior of returning
nil for the event on error.
In `@pkg/redo/writer/memory/dml_writer_test.go`:
- Around line 27-44: Add test coverage that exercises the DMLWriter's runtime
behavior: after creating the writer with NewDMLWriter, start its Run method in a
goroutine, call AppendDMLEvents with a few synthetic DML events, wait for
processing (e.g., via a done channel, context cancellation, or Checkpoint/ack
mechanism exposed by the writer), then assert expected side-effects and call
Close; reference NewDMLWriter, Run, AppendDMLEvents, and Close to locate the
code paths to exercise and synchronize on (use whatever internal signaling the
DMLWriter exposes for testing if available).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d399c8e1-948c-47fa-b3f3-c13908ecf151
📒 Files selected for processing (10)
pkg/redo/writer/blackhole/blackhole_log_writer.gopkg/redo/writer/file/file_log_writer.gopkg/redo/writer/file/file_log_writer_test.gopkg/redo/writer/memory/ddl_writer.gopkg/redo/writer/memory/ddl_writer_test.gopkg/redo/writer/memory/dml_writer.gopkg/redo/writer/memory/dml_writer_test.gopkg/redo/writer/memory/encoding_worker.gopkg/redo/writer/memory/file_worker.gopkg/redo/writer/memory/mem_log_writer.go
💤 Files with no reviewable changes (1)
- pkg/redo/writer/memory/mem_log_writer.go
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/redo/writer/file/file_log_writer_test.go
- pkg/redo/writer/blackhole/blackhole_log_writer.go
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/redo/writer/blackhole/writer.go (1)
55-60: Consider wrapping context error for stack trace.
ctx.Err()is returned directly without wrapping. Per coding guidelines, errors from external sources should be wrapped witherrors.Trace()to attach a stack trace for debugging.♻️ Proposed fix
+import ( + "context" + + "github.com/pingcap/log" + "github.com/pingcap/ticdc/pkg/common/event" + "github.com/pingcap/ticdc/pkg/errors" + "github.com/pingcap/ticdc/pkg/redo/writer" + "go.uber.org/zap" +)func (bs *blackHoleDMLWriter) Run(ctx context.Context) error { select { case <-ctx.Done(): - return ctx.Err() + return errors.Trace(ctx.Err()) } }As per coding guidelines: "When an error comes from a third-party or library call in Go, wrap it immediately with
errors.Trace(err)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/redo/writer/blackhole/writer.go` around lines 55 - 60, The Run method on blackHoleDMLWriter returns ctx.Err() directly; wrap that external context error with errors.Trace for stack information by replacing return ctx.Err() with return errors.Trace(ctx.Err()) in blackHoleDMLWriter.Run (the select on ctx.Done()/ctx.Err()). Also ensure the package imports the errors package that provides Trace if not already imported.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/redo/writer/blackhole/writer.go`:
- Around line 55-60: The Run method on blackHoleDMLWriter returns ctx.Err()
directly; wrap that external context error with errors.Trace for stack
information by replacing return ctx.Err() with return errors.Trace(ctx.Err()) in
blackHoleDMLWriter.Run (the select on ctx.Done()/ctx.Err()). Also ensure the
package imports the errors package that provides Trace if not already imported.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f4bdd6c0-104f-4046-b253-0bf5fb80dbb1
📒 Files selected for processing (7)
downstreamadapter/sink/redo/sink.godownstreamadapter/sink/redo/sink_test.gopkg/redo/writer/blackhole/writer.gopkg/redo/writer/file/file_log_writer.gopkg/redo/writer/memory/dml_writer.gopkg/redo/writer/writer.gopkg/redo/writer/writer_mock.go
🚧 Files skipped from review as they are similar to previous changes (2)
- downstreamadapter/sink/redo/sink_test.go
- pkg/redo/writer/writer.go
…erface' into refactor-writer-interface
ti-chi-bot
Bot
added
release-note-none
|
/test all |
|
/test all |
|
/retest |
ti-chi-bot
Bot
added
needs-1-more-lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: asddongmen, wk989898 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ti-chi-bot
Bot
added
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
|
/retest |
1 similar comment
|
/retest |
|
/test pull-cdc-mysql-integration-light-next-gen |
|
/retest |
|
/test all |
|
/retest |
|
/test all |
|
/retest |
|
/test pull-cdc-mysql-integration-light-next-gen |
1 similar comment
|
/test pull-cdc-mysql-integration-light-next-gen |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
/override pull-cdc-mysql-integration-light-next-gen |
|
@tenfyzhong: Overrode contexts on behalf of tenfyzhong: pull-cdc-mysql-integration-light-next-gen DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
5 participants
What problem does this PR solve?
Issue Number: close #4590
What is changed and how it works?
Check List
Tests
Questions
Will it cause performance regression or break compatibility?
Do you need to update user documentation, design documentation or monitoring documentation?
Release note
Summary by CodeRabbit