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

systemlog indexer and api #1960

Merged
merged 5 commits into from Mar 11, 2020
Merged

systemlog indexer and api #1960

merged 5 commits into from Mar 11, 2020

Conversation

Frankonly
Copy link
Contributor

No description provided.

@Frankonly Frankonly force-pushed the systemlog branch 3 times, most recently from c3fcdf6 to 5b1408b Compare March 9, 2020 14:01
@codecov
Copy link

codecov bot commented Mar 9, 2020

Codecov Report

Merging #1960 into master will increase coverage by 0.19%.
The diff coverage is 61.53%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1960      +/-   ##
==========================================
+ Coverage   58.13%   58.32%   +0.19%     
==========================================
  Files         186      185       -1     
  Lines       16143    16089      -54     
==========================================
  Hits         9384     9384              
+ Misses       5592     5518      -74     
- Partials     1167     1187      +20
Impacted Files Coverage Δ
config/config.go 79.57% <ø> (ø) ⬆️
api/api.go 49.47% <0%> (-0.53%) ⬇️
blockchain/blockdao/blockdao.go 45.67% <33.33%> (+0.36%) ⬆️
systemlog/systemlog.go 72.8% <85.18%> (+72.8%) ⬆️
action/protocol/staking/validations.go 15.59% <0%> (-83.44%) ⬇️
db/trie/extensionnode.go 57.62% <0%> (-1.7%) ⬇️
db/trie/branchnode.go 65.59% <0%> (-1.61%) ⬇️
action/protocol/staking/candidate_center.go 92.06% <0%> (-0.25%) ⬇️
action/protocol/staking/protocol.go 12.19% <0%> (ø) ⬆️
ioctl/newcmd/bc/bc.go
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f027c85...a35bfdb. Read the comment docs.

@Frankonly Frankonly marked this pull request as ready for review March 9, 2020 15:13
@Frankonly Frankonly requested a review from a team as a code owner March 9, 2020 15:13
Copy link
Collaborator

@CoderZhi CoderZhi left a comment

Choose a reason for hiding this comment

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

almost there

} else {
dao = blockdao.NewBlockDAO(kvStore, nil, cfg.Chain.CompressBlock, cfg.DB)
}
dao := blockdao.NewBlockDAO(kvStore, indexers, cfg.Chain.CompressBlock, cfg.DB)
Copy link
Collaborator

Choose a reason for hiding this comment

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

indexers are appended to blockdao in sync mode. In async mode, we need to add the indexers as subscriber. So you need to revert this change.

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 handle this at line 163

if !cfg.Chain.EnableAsyncIndexWrite {
	indexers = append(indexers, indexer)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or we need to make systemlog indexer as subsrciber?

systemlog/systemlog.go Outdated Show resolved Hide resolved
@Frankonly Frankonly force-pushed the systemlog branch 2 times, most recently from 70ee38e to ae13563 Compare March 10, 2020 05:27
Comment on lines 185 to +186
var dao blockdao.BlockDAO
if gateway && !cfg.Chain.EnableAsyncIndexWrite {
dao = blockdao.NewBlockDAO(kvStore, []blockdao.BlockIndexer{indexer}, cfg.Chain.CompressBlock, cfg.DB)
} else {
dao = blockdao.NewBlockDAO(kvStore, nil, cfg.Chain.CompressBlock, cfg.DB)
}
dao = blockdao.NewBlockDAO(kvStore, indexers, cfg.Chain.CompressBlock, cfg.DB)
Copy link
Collaborator

Choose a reason for hiding this comment

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

one line

@CoderZhi
Copy link
Collaborator

It is better to have system log indexer and the action indexer sharing the same config, EnableAsyncIndexing.

@CoderZhi CoderZhi merged commit c17caf5 into iotexproject:master Mar 11, 2020
@Frankonly
Copy link
Contributor Author

#1346

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

2 participants