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.

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

Introduce pull based replicator that does not use kafka #2377

Merged
merged 50 commits into from Aug 15, 2019

Conversation

meiliang86
Copy link
Contributor

@meiliang86 meiliang86 commented Aug 12, 2019

Domain replication is still going through kafka, which will be addressed in a follow up PR.

client/clientBean.go Outdated Show resolved Hide resolved
@@ -643,3 +643,18 @@ func (c *clientImpl) getRandomClient() (workflowserviceclient.Interface, error)

return client.(workflowserviceclient.Interface), nil
}

func (c *clientImpl) GetReplicationTasks(
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: GetReplicationTasks -> GetReplicationMessages

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I need lots of suggestion on names. Please keep them coming :)

@@ -246,6 +255,14 @@ type (
RPCAddress string `yaml:"rpcAddress"`
}

// ReplicationConsumerConfig contains config for replication consumer
ReplicationConsumerConfig struct {
Copy link
Contributor

@wxing1292 wxing1292 Aug 12, 2019

Choose a reason for hiding this comment

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

are we going to officially support 2 means to deliver replication messages?

I hope not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we need to support both. This is mainly for migration and testing purpose. Once we migrate to the new model we can completely remove kafka related stuffs.

Copy link
Contributor

Choose a reason for hiding this comment

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

then i guess, the preference is to use dynamic config, not this.

@longquanzheng longquanzheng self-requested a review August 14, 2019 21:41
@@ -23,6 +23,7 @@ package history
import (
"context"
"fmt"
"github.com/golang/mock/gomock"
Copy link
Contributor

Choose a reason for hiding this comment

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

fmt

@@ -21,6 +21,7 @@
package history

import (
"github.com/golang/mock/gomock"
Copy link
Contributor

Choose a reason for hiding this comment

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

fmt

@@ -21,6 +21,7 @@
package history

import (
"github.com/golang/mock/gomock"
Copy link
Contributor

Choose a reason for hiding this comment

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

fmt

@@ -21,6 +21,7 @@
package history

import (
"github.com/golang/mock/gomock"
Copy link
Contributor

Choose a reason for hiding this comment

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

fmt

@@ -22,6 +22,7 @@ package history

import (
"fmt"
"github.com/golang/mock/gomock"
Copy link
Contributor

Choose a reason for hiding this comment

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

fmt

@@ -22,6 +22,7 @@ package history

import (
"errors"
"github.com/golang/mock/gomock"
Copy link
Contributor

Choose a reason for hiding this comment

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

fmt

token: &r.ReplicationToken{ShardID: common.Int32Ptr(int32(p.shard.GetShardID())), TaskID: common.Int64Ptr(p.readLevel)},
token: &r.ReplicationToken{
ShardID: common.Int32Ptr(int32(p.shard.GetShardID())),
LastRetrivedMessageId: common.Int64Ptr(p.lastProcessedMessageID),
Copy link
Contributor

Choose a reason for hiding this comment

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

lastRetrievedMessageID

@@ -23,6 +23,7 @@ package history
import (
ctx "context"
"errors"
"github.com/golang/mock/gomock"
Copy link
Contributor

Choose a reason for hiding this comment

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

fmt

@@ -24,6 +24,7 @@ import (
"context"
"encoding/json"
"errors"
"github.com/golang/mock/gomock"
Copy link
Contributor

Choose a reason for hiding this comment

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

fmt

@@ -22,6 +22,7 @@ package history

import (
"context"
"github.com/golang/mock/gomock"
Copy link
Contributor

Choose a reason for hiding this comment

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

fmt

@@ -24,6 +24,7 @@ import (
"context"
"encoding/json"
"errors"
"github.com/golang/mock/gomock"
Copy link
Contributor

Choose a reason for hiding this comment

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

fmt

@@ -21,6 +21,7 @@
package history

import (
"github.com/golang/mock/gomock"
Copy link
Contributor

Choose a reason for hiding this comment

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

fmt

@meiliang86 meiliang86 merged commit 602b810 into master Aug 15, 2019
@wxing1292 wxing1292 deleted the pull-replicator branch August 21, 2019 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants