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

*: fix data race in the cache #294

Merged
merged 2 commits into from
Apr 3, 2022
Merged

*: fix data race in the cache #294

merged 2 commits into from
Apr 3, 2022

Conversation

hawkingrei
Copy link

Signed-off-by: Weizhen Wang [email protected]

we find the data race in the cache of ristretto when to test our cases.

==================
WARNING: DATA RACE
Write at 0x00c002c1a0d8 by goroutine 99:
  github.com/dgraph-io/ristretto.(*Cache).Close()
      /home/prow/go/pkg/mod/github.com/dgraph-io/[email protected]/cache.go:363 +0xe4
  github.com/pingcap/tidb/store/copr.(*Store).Close()
      /go/tidb/store/copr/store.go:97 +0x91
  github.com/pingcap/tidb/store/mockstore/mockstorage.(*mockStorage).Close()
      /go/tidb/store/mockstore/mockstorage/storage.go:116 +0x27
  github.com/pingcap/tidb/testkit.bootstrap.func1()
      /go/tidb/testkit/mockstore.go:57 +0x68
  runtime.deferreturn()
      /usr/local/go/src/runtime/panic.go:436 +0x32
  github.com/pingcap/failpoint.parseTerm()
      /home/prow/go/pkg/mod/github.com/pingcap/[email protected]/terms.go:149 +0x377
  github.com/pingcap/failpoint.parse()
      /home/prow/go/pkg/mod/github.com/pingcap/[email protected]/terms.go:126 +0xa8
  github.com/pingcap/failpoint.newTerms()
      /home/prow/go/pkg/mod/github.com/pingcap/[email protected]/terms.go:98 +0x50
  github.com/pingcap/failpoint.(*Failpoint).Enable()
      /home/prow/go/pkg/mod/github.com/pingcap/[email protected]/failpoint.go:54 +0x44
  github.com/pingcap/failpoint.(*Failpoints).Enable()
      /home/prow/go/pkg/mod/github.com/pingcap/[email protected]/failpoints.go:105 +0x276
  github.com/pingcap/failpoint.Enable()
      /home/prow/go/pkg/mod/github.com/pingcap/[email protected]/failpoints.go:225 +0x14f
  github.com/pingcap/tidb/executor/seqtest_test.TestParallelHashAggClose()
      /go/tidb/executor/seqtest/seq_executor_test.go:711 +0x150
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1439 +0x213
  testing.(*T).Run.func1()
      /usr/local/go/src/testing/testing.go:1486 +0x47
Previous read at 0x00c002c1a0d8 by goroutine 7:
  github.com/dgraph-io/ristretto.(*Cache).Get()
      /home/prow/go/pkg/mod/github.com/dgraph-io/[email protected]/cache.go:225 +0x4a
  github.com/pingcap/tidb/store/copr.(*coprCache).Get()
      /go/tidb/store/copr/coprocessor_cache.go:152 +0x84
  github.com/pingcap/tidb/store/copr.(*copIteratorWorker).handleTaskOnce()
      /go/tidb/store/copr/coprocessor.go:724 +0x744
  github.com/pingcap/tidb/store/copr.(*copIteratorWorker).handleTask()
      /go/tidb/store/copr/coprocessor.go:676 +0x1e5
  github.com/pingcap/tidb/store/copr.(*copIteratorWorker).run()
      /go/tidb/store/copr/coprocessor.go:418 +0x173
  github.com/pingcap/tidb/store/copr.(*copIterator).open.func1()
      /go/tidb/store/copr/coprocessor.go:450 +0x58
Goroutine 99 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:1486 +0x724
  testing.runTests.func1()
      /usr/local/go/src/testing/testing.go:1839 +0x99
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1439 +0x213
  testing.runTests()
      /usr/local/go/src/testing/testing.go:1837 +0x7e4
  testing.(*M).Run()
      /usr/local/go/src/testing/testing.go:1719 +0xa71
  go.uber.org/goleak.VerifyTestMain()
      /home/prow/go/pkg/mod/go.uber.org/[email protected]/testmain.go:53 +0x59
  github.com/pingcap/tidb/executor/seqtest.TestMain()
      /go/tidb/executor/seqtest/main_test.go:37 +0x430
  main.main()
      _testmain.go:125 +0x317
Goroutine 7 (finished) created at:
  github.com/pingcap/tidb/store/copr.(*copIterator).open()
      /go/tidb/store/copr/coprocessor.go:450 +0xc4
  github.com/pingcap/tidb/store/copr.(*CopClient).Send()
      /go/tidb/store/copr/coprocessor.go:143 +0x10c9
  github.com/pingcap/tidb/distsql.Select()
      /go/tidb/distsql/distsql.go:101 +0x8c3
  github.com/pingcap/tidb/distsql.SelectWithRuntimeStats()
      /go/tidb/distsql/distsql.go:150 +0xbb
  github.com/pingcap/tidb/executor.selectResultHook.SelectResult()
      /go/tidb/executor/table_reader.go:53 +0x1c5
  github.com/pingcap/tidb/executor.(*TableReaderExecutor).buildResp()
      /go/tidb/executor/table_reader.go:310 +0x69d
  github.com/pingcap/tidb/executor.(*TableReaderExecutor).Open()
      /go/tidb/executor/table_reader.go:210 +0x13f5
  github.com/pingcap/tidb/executor.(*baseExecutor).Open()
      /go/tidb/executor/executor.go:185 +0x6a9
  github.com/pingcap/tidb/executor.(*HashAggExec).Open()
      /go/tidb/executor/aggregate.go:309 +0xe6
  github.com/pingcap/tidb/executor.(*ExecStmt).Exec()
      /go/tidb/executor/adapter.go:407 +0x7f8
  github.com/pingcap/tidb/session.runStmt()
      /go/tidb/session/session.go:2017 +0x6cb
  github.com/pingcap/tidb/session.(*session).ExecuteStmt()
      /go/tidb/session/session.go:1894 +0xd3a
  github.com/pingcap/tidb/session.(*session).Execute()
      /go/tidb/session/session.go:1534 +0x478
  github.com/pingcap/tidb/executor/seqtest_test.TestParallelHashAggClose()
      /go/tidb/executor/seqtest/seq_executor_test.go:716 +0x21b
  github.com/pingcap/failpoint.parseTerm()
      /home/prow/go/pkg/mod/github.com/pingcap/[email protected]/terms.go:149 +0x377
  github.com/pingcap/failpoint.parse()
      /home/prow/go/pkg/mod/github.com/pingcap/[email protected]/terms.go:126 +0xa8
  github.com/pingcap/failpoint.newTerms()
      /home/prow/go/pkg/mod/github.com/pingcap/[email protected]/terms.go:98 +0x50
  github.com/pingcap/failpoint.(*Failpoint).Enable()
      /home/prow/go/pkg/mod/github.com/pingcap/[email protected]/failpoint.go:54 +0x44
  github.com/pingcap/failpoint.(*Failpoints).Enable()
      /home/prow/go/pkg/mod/github.com/pingcap/[email protected]/failpoints.go:105 +0x276
  github.com/pingcap/failpoint.Enable()
      /home/prow/go/pkg/mod/github.com/pingcap/[email protected]/failpoints.go:225 +0x14f
  github.com/pingcap/tidb/executor/seqtest_test.TestParallelHashAggClose()
      /go/tidb/executor/seqtest/seq_executor_test.go:711 +0x150
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1439 +0x213
  testing.(*T).Run.func1()
      /usr/local/go/src/testing/testing.go:1486 +0x47
==================

ref pingcap/tidb#33649

@hawkingrei
*: fix data race in the cache
05f4911 
Signed-off-by: Weizhen Wang <[email protected]>
@CLAassistant
Copy link

CLAassistant commented Apr 2, 2022 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@hawkingrei
Copy link
Author

hawkingrei commented Apr 2, 2022 •
edited
Loading

@NamanJain8 PTAL

NamanJain8
NamanJain8 approved these changes Apr 3, 2022
View reviewed changes
Copy link
Contributor

@NamanJain8 NamanJain8 left a comment

Choose a reason for hiding this comment

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

LGTM

@hawkingrei
Copy link
Author

@NamanJain8 I see that this repository is updated a bit infrequently. so will Dgraph continue to develop it after now?

@hawkingrei
Copy link
Author

hawkingrei commented Apr 3, 2022 •
edited
Loading

@NamanJain8 Can you merge it?

@akon-dey akon-dey merged commit 8e850b7 into dgraph-io:master Apr 3, 2022
@hawkingrei hawkingrei deleted the fix_data_race branch April 3, 2022 17:02
@stlava stlava mentioned this pull request Apr 9, 2024
Closed
@StarpTech StarpTech mentioned this pull request Jul 15, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NamanJain8 NamanJain8 NamanJain8 approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hawkingrei @CLAassistant @NamanJain8 @akon-dey