Skip to content

Commit

Permalink
feat(varlogtest): create new instance of admin and log (#591)
Browse files Browse the repository at this point in the history
### What this PR does

This PR renames `pkg/varlogtest.(*VarlogTest).Admin` and `pkg/varlogtest.(*VarlogTest).Log` to `pkg/varlogtest.(*VarlogTest).NewAdminClient` and `pkg/varlogtest.(*VarlogTest).NewLogClient` respectively. It also makes them return a new instance of the admin client and the log client when invoked.
Previously, they returned the same instances, so callers shared them. However, it wasn't the right decision because the callers had to share the clients regardless of their needs.
  • Loading branch information
ijsong authored Oct 4, 2023
2 parents 626bdf8 + 2c98f7f commit 385446d
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 26 deletions.
7 changes: 4 additions & 3 deletions pkg/varlogtest/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,15 @@ import (
)

type testAdmin struct {
vt *VarlogTest
vt *VarlogTest
closed bool
}

var _ varlog.Admin = (*testAdmin)(nil)

func (c *testAdmin) lock() error {
c.vt.cond.L.Lock()
if c.vt.adminClientClosed {
if c.closed {
c.vt.cond.L.Unlock()
return verrors.ErrClosed
}
Expand Down Expand Up @@ -534,6 +535,6 @@ func (c *testAdmin) RemoveMRPeer(ctx context.Context, raftURL string, opts ...va
func (c *testAdmin) Close() error {
c.vt.cond.L.Lock()
defer c.vt.cond.L.Unlock()
c.vt.adminClientClosed = true
c.closed = true
return nil
}
9 changes: 5 additions & 4 deletions pkg/varlogtest/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,15 @@ import (
)

type testLog struct {
vt *VarlogTest
vt *VarlogTest
closed bool
}

var _ varlog.Log = (*testLog)(nil)

func (c *testLog) lock() error {
c.vt.cond.L.Lock()
if c.vt.varlogClientClosed {
if c.closed {
c.vt.cond.L.Unlock()
return verrors.ErrClosed
}
Expand All @@ -35,7 +36,7 @@ func (c *testLog) unlock() {
func (c *testLog) Close() error {
c.vt.cond.L.Lock()
defer c.vt.cond.L.Unlock()
c.vt.varlogClientClosed = true
c.closed = true
c.vt.cond.Broadcast()
return nil
}
Expand Down Expand Up @@ -247,7 +248,7 @@ func (c *testLog) SubscribeTo(ctx context.Context, topicID types.TopicID, logStr
return logEntries
}
s.vt.closedClient = func() bool {
return c.vt.varlogClientClosed
return c.closed
}

s.wg.Add(1)
Expand Down
16 changes: 4 additions & 12 deletions pkg/varlogtest/varlogtest.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ import (
type VarlogTest struct {
config

admin *testAdmin
vlg *testLog

rng *rand.Rand

mu sync.Mutex
Expand All @@ -38,9 +35,6 @@ type VarlogTest struct {
nextTopicID types.TopicID
nextStorageNodeID types.StorageNodeID
nextLogStreamID types.LogStreamID

adminClientClosed bool
varlogClientClosed bool
}

func New(opts ...Option) (*VarlogTest, error) {
Expand All @@ -62,8 +56,6 @@ func New(opts ...Option) (*VarlogTest, error) {
leaderMR: types.InvalidNodeID,
}
vt.cond = sync.NewCond(&vt.mu)
vt.admin = &testAdmin{vt: vt}
vt.vlg = &testLog{vt: vt}

for _, mrn := range vt.initialMRNodes {
if vt.leaderMR == types.InvalidNodeID {
Expand All @@ -76,12 +68,12 @@ func New(opts ...Option) (*VarlogTest, error) {
return vt, nil
}

func (vt *VarlogTest) Admin() varlog.Admin {
return vt.admin
func (vt *VarlogTest) NewAdminClient() varlog.Admin {
return &testAdmin{vt: vt}
}

func (vt *VarlogTest) Log() varlog.Log {
return vt.vlg
func (vt *VarlogTest) NewLogClient() varlog.Log {
return &testLog{vt: vt}
}

func (vt *VarlogTest) generateTopicID() types.TopicID {
Expand Down
14 changes: 7 additions & 7 deletions pkg/varlogtest/varlogtest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ func TestVarlotTest_LogStreamAppender(t *testing.T) {
)
require.NoError(t, err)

adm := vt.Admin()
vlg := vt.Log()
adm := vt.NewAdminClient()
vlg := vt.NewLogClient()
defer func() {
require.NoError(t, vlg.Close())
require.NoError(t, adm.Close())
Expand Down Expand Up @@ -176,8 +176,8 @@ func TestVarlogTest(t *testing.T) {
)
require.NoError(t, err)

adm := vt.Admin()
vlg := vt.Log()
adm := vt.NewAdminClient()
vlg := vt.NewLogClient()
defer func() {
require.NoError(t, vlg.Close())
require.NoError(t, adm.Close())
Expand Down Expand Up @@ -594,8 +594,8 @@ func TestVarlogTest_Trim(t *testing.T) {
)
require.NoError(t, err)

adm := vt.Admin()
vlg := vt.Log()
adm := vt.NewAdminClient()
vlg := vt.NewLogClient()
defer func() {
require.NoError(t, vlg.Close())
require.NoError(t, adm.Close())
Expand Down Expand Up @@ -783,7 +783,7 @@ func TestVarlogTestAdminMetadataRepository(t *testing.T) {
)
require.NoError(t, err)

tc.testf(t, vt.Admin())
tc.testf(t, vt.NewAdminClient())
})
}
}
Expand Down

0 comments on commit 385446d

Please sign in to comment.