Skip to content
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.

Sign up for GitHub

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

Jump to bottom

enhance: wal interface definition #33745

Merged
merged 3 commits into from
Jun 24, 2024
Merged

enhance: wal interface definition #33745

merged 3 commits into from
Jun 24, 2024

Conversation

chyezh
Copy link
Contributor

@chyezh chyezh commented Jun 11, 2024

issue: #33285

@sre-ci-robot sre-ci-robot added size/XXL Denotes a PR that changes 1000+ lines. area/compilation area/dependency Pull requests that update a dependency file labels Jun 11, 2024
@mergify mergify bot added dco-passed DCO check passed. kind/enhancement Issues or changes related to enhancement labels Jun 11, 2024
@chyezh chyezh force-pushed the feat_log_service_wal_commit branch from e19fe83 to b839530 Compare June 11, 2024 03:24
@chyezh
Copy link
Contributor Author

chyezh commented Jun 11, 2024

WAL

wal package is the basic defination of wal interface of milvus lognode.

Project arrangement

  • /: only define exposed interfaces.
  • /walimpls/: define the underlying message system interfaces need to be implemented.
  • /registry/: A static lifetime registry to regsiter new implementation for inverting dependency.
  • /adaptor/: adaptors to implement wal interface from walimpls interface
  • /helper/: A utility used to help developer to implement walimpls conveniently.
  • /utility/: A utility code for common logic or data structure.

Lifetime Of Interfaces

  • OpenerBuilder has a static lifetime in a programs:
  • Opener keep same lifetime with underlying resources (such as mq client).
  • WAL keep same lifetime with underlying writer of wal, and it's lifetime is always included in related Opener.
  • Scanner keep same lifetime with underlying reader of wal, and it's lifetime is always included in related WAL.

Add New Implemetation Of WAL

developper who want to add a new implementation of wal should implements the walimpls package interfaces. following interfaces is required:

  • walimpls.OpenerBuilderImpls
  • walimpls.OpenerImpls
  • walimpls.ScannerImpls
  • walimpls.WALImpls

OpenerBuilderImpls create OpenerImpls; OpenerImpls creates WALImpls; WALImpls create ScannerImpls.
Then register the implmentation of walimpls.OpenerBuilderImpls into registry package.

var _ OpenerBuilderImpls = b{};
registry.RegisterBuilder(b{})

All things have been done.

Use WAL

name := "your builder name"
var yourCh *logpb.PChannelInfo
opener, err := registry.MustGetBuilder(name).Build()
if err != nil {
    panic(err)
}
ctx := context.Background()
logger, err := opener.Open(ctx, wal.OpenOption{
    Channel: yourCh  
})
if err != nil {
    panic(err)
}

Adaptor

package adaptor is used to adapt walimpls and wal together.
common wal function should be implement by it. Such as:

  • lifetime management
  • interceptor implementation
  • scanner wrapped up
  • write ahead cache implementation

@mergify Mergify
Copy link
Contributor

mergify bot commented Jun 11, 2024

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@chyezh
Copy link
Contributor Author

chyezh commented Jun 11, 2024

/run-cpu-e2e

@chyezh
Copy link
Contributor Author

chyezh commented Jun 11, 2024

rerun ut

2 similar comments
@chyezh
Copy link
Contributor Author

chyezh commented Jun 12, 2024

rerun ut

@chyezh
Copy link
Contributor Author

chyezh commented Jun 12, 2024

rerun ut

@chyezh chyezh force-pushed the feat_log_service_wal_commit branch 2 times, most recently from 6804b15 to 04e6182 Compare June 13, 2024 01:14
@mergify Mergify
Copy link
Contributor

mergify bot commented Jun 13, 2024

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@chyezh
Copy link
Contributor Author

chyezh commented Jun 13, 2024

/run-cpu-e2e

@mergify Mergify
Copy link
Contributor

mergify bot commented Jun 13, 2024

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@chyezh
Copy link
Contributor Author

chyezh commented Jun 13, 2024

/run-cpu-e2e

@chyezh
Copy link
Contributor Author

chyezh commented Jun 13, 2024

rerun ut

@chyezh chyezh force-pushed the feat_log_service_wal_commit branch from 04e6182 to 3325905 Compare June 13, 2024 03:46
@chyezh
Copy link
Contributor Author

chyezh commented Jun 13, 2024

rerun ut

1 similar comment
@chyezh
Copy link
Contributor Author

chyezh commented Jun 13, 2024

rerun ut

@chyezh
enhance: wal interface definition
60be4f7 
Signed-off-by: chyezh <chyezh@outlook.com>
@chyezh chyezh force-pushed the feat_log_service_wal_commit branch 2 times, most recently from 931deaf to 417e931 Compare June 13, 2024 08:58
@mergify Mergify
Copy link
Contributor

mergify bot commented Jun 13, 2024

@chyezh ut workflow job failed, comment rerun ut can trigger the job again.

@chyezh chyezh force-pushed the feat_log_service_wal_commit branch from 417e931 to c55a9c4 Compare June 13, 2024 09:18
@mergify Mergify
Copy link
Contributor

mergify bot commented Jun 13, 2024

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@chyezh
Copy link
Contributor Author

chyezh commented Jun 14, 2024

/run-cpu-e2e

@chyezh
enhance: add walimplstest
825a845 
Signed-off-by: chyezh <chyezh@outlook.com>
@chyezh chyezh force-pushed the feat_log_service_wal_commit branch from c55a9c4 to 825a845 Compare June 14, 2024 01:03
@mergify Mergify
Copy link
Contributor

mergify bot commented Jun 14, 2024

@chyezh ut workflow job failed, comment rerun ut can trigger the job again.

@chyezh
Copy link
Contributor Author

chyezh commented Jun 14, 2024

rerun ut

@mergify mergify bot added the ci-passed label Jun 14, 2024
@jaime0815
Copy link
Contributor

/lgtm

@sre-ci-robot sre-ci-robot added lgtm and removed lgtm labels Jun 14, 2024
@mergify mergify bot removed the ci-passed label Jun 19, 2024
@mergify Mergify
Copy link
Contributor

mergify bot commented Jun 19, 2024

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@chyezh
Copy link
Contributor Author

chyezh commented Jun 19, 2024

/run-cpu-e2e

@chyezh
Copy link
Contributor Author

chyezh commented Jun 19, 2024

rerun ut

1 similar comment
@chyezh
Copy link
Contributor Author

chyezh commented Jun 19, 2024

rerun ut

@chyezh
enhance: rename log as streaming
1386651 
Signed-off-by: chyezh <chyezh@outlook.com>
@chyezh chyezh force-pushed the feat_log_service_wal_commit branch from 91e5586 to 1386651 Compare June 19, 2024 06:14
@chyezh chyezh mentioned this pull request Jun 19, 2024
Open
1 task
@mergify Mergify
Copy link
Contributor

mergify bot commented Jun 19, 2024

@chyezh ut workflow job failed, comment rerun ut can trigger the job again.

@chyezh
Copy link
Contributor Author

chyezh commented Jun 19, 2024

rerun ut

@mergify mergify bot added the ci-passed label Jun 19, 2024
@chyezh chyezh mentioned this pull request Jun 22, 2024
Open
40 tasks
congqixia
congqixia approved these changes Jun 24, 2024
View reviewed changes
Copy link
Contributor

@congqixia congqixia left a comment

Choose a reason for hiding this comment

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

overall lgtm, please sync WAL & WALImpl interface in next PRs
/lgtm

internal/streamingnode/server/wal/walimpls/wal.go
Channel() *streamingpb.PChannelInfo

// Append writes a record to the log.
Append(ctx context.Context, msg message.MutableMessage) (message.MessageID, error)
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like WALImpls missing AppendAsync method def

Copy link
Contributor Author

Choose a reason for hiding this comment

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

WALImpls is not required to implement AppendAsync.
WAL will give a adaptor to auto implement it.

@sre-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chyezh, congqixia

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sre-ci-robot sre-ci-robot merged commit b923728 into milvus-io:master Jun 24, 2024
15 checks passed
@chyezh chyezh deleted the feat_log_service_wal_commit branch June 24, 2024 05:51
yellow-shine pushed a commit to yellow-shine/milvus that referenced this pull request Jul 2, 2024
@chyezh @yellow-shine
enhance: wal interface definition (milvus-io#33745)
066d21a 
issue: milvus-io#33285

---------

Signed-off-by: chyezh <chyezh@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@congqixia congqixia congqixia approved these changes

@sunby sunby Awaiting requested review from sunby

@xige-16 xige-16 Awaiting requested review from xige-16

Assignees

@jaime0815 jaime0815

@congqixia congqixia

Labels
approved area/compilation area/dependency Pull requests that update a dependency file ci-passed dco-passed DCO check passed. kind/enhancement Issues or changes related to enhancement lgtm size/XXL Denotes a PR that changes 1000+ lines.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@chyezh @jaime0815 @sre-ci-robot @congqixia