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

tsm1 meta lint #4460

Merged
merged 1 commit into from
Oct 15, 2015
Merged

tsm1 meta lint #4460

merged 1 commit into from
Oct 15, 2015

Conversation

benbjohnson
Copy link
Contributor

Overview

This pull request fixes a bunch of warnings reported by gometalinter. The gometalinter output was also excessive so I moved a subset of the functionality into a Makefile with the following targets:

deadcode
cyclo
aligncheck
defercheck
structcheck
lint
errcheck

metalint # runs all the above

Cyclomatic Complexity

Within tsm1, there 26 functions which have a cyclomatic complexity score higher than 10:

39 tsm1 (*Engine).Compact tsdb/engine/tsm1/tsm1.go:507:1
35 tsm1 (*Engine).rewriteFile tsdb/engine/tsm1/tsm1.go:988:1
23 tsm1 (*Log).flush tsdb/engine/tsm1/wal.go:516:1
21 tsm1 (*Engine).convertKeysAndWriteMetadata tsdb/engine/tsm1/tsm1.go:811:1
21 tsm1 (*cursor).SeekTo tsdb/engine/tsm1/cursor.go:213:1
19 tsm1 (*Log).readFileToCache tsdb/engine/tsm1/wal.go:331:1
15 tsm1 (*Engine).flushDeletes tsdb/engine/tsm1/tsm1.go:1189:1
14 tsm1 (*Engine).Write tsdb/engine/tsm1/tsm1.go:351:1
14 tsm1 (*FloatDecoder).Next tsdb/engine/tsm1/float.go:152:1
12 tsm1 (*multiFieldCursor).read tsdb/engine/tsm1/cursor.go:136:1
12 tsm1 (*Log).WritePoints tsdb/engine/tsm1/wal.go:195:1
12 tsm1 (*Engine).filterDataBetweenTimes tsdb/engine/tsm1/tsm1.go:944:1
11 tsm1 (*Log).addToCache tsdb/engine/tsm1/wal.go:253:1
11 tsm1 (*Engine).Open tsdb/engine/tsm1/tsm1.go:210:1
36 tsm1_test TestEngine_SupportMultipleFields tsdb/engine/tsm1/tsm1_test.go:443:1
25 tsm1_test TestLog_TestWriteQueryOpen tsdb/engine/tsm1/wal_test.go:21:1
23 tsm1_test TestEngine_Deletes tsdb/engine/tsm1/tsm1_test.go:1074:1
22 tsm1_test TestEngine_WriteAndReadFloats tsdb/engine/tsm1/tsm1_test.go:19:1
17 tsm1_test TestEngine_CompactWithSeriesInOneFile tsdb/engine/tsm1/tsm1_test.go:764:1
15 tsm1_test TestEngine_Compaction tsdb/engine/tsm1/tsm1_test.go:281:1
13 tsm1_test TestEngine_IndexGoodAfterFlush tsdb/engine/tsm1/tsm1_test.go:1239:1
13 tsm1_test TestEngine_WriteIntoCompactedFile tsdb/engine/tsm1/tsm1_test.go:977:1
12 tsm1_test TestEngine_CursorDescendingOrder tsdb/engine/tsm1/tsm1_test.go:689:1
12 tsm1_test TestEngine_CompactionWithCopiedBlocks tsdb/engine/tsm1/tsm1_test.go:855:1
11 tsm1_test TestLog_Quick tsdb/engine/tsm1/wal_test.go:149:1
11 tsm1_test TestEngine_KeyCollisionsAreHandled tsdb/engine/tsm1/tsm1_test.go:362:1

Paul is currently in the compaction code but I think it'd be worth it to break that code up once he's done.

@otoolep
Copy link
Contributor

otoolep commented Oct 15, 2015

You mention a Makefile, but I see no Makefile in the patch. Did you mean to add it?

@@ -113,7 +113,7 @@ func TestEngine_WriteAndReadFloats(t *testing.T) {
}
tx.Rollback()

if err := e.Close(); err != nil {
if err := e.Engine.Close(); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does it flag 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 renamed Cleanup() to Close() on the test engine wrapper. There were tests that depended on calling the underlying Engine's Close() directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, yes, makes sense -- thanks.

@otoolep
Copy link
Contributor

otoolep commented Oct 15, 2015

Some nice cleanups. +1 on green.

@benbjohnson
Copy link
Contributor Author

The Makefile was in the .gitignore. Fixed in c27f8ae.

benbjohnson added a commit that referenced this pull request Oct 15, 2015
@benbjohnson benbjohnson merged commit 11cdb37 into influxdata:master Oct 15, 2015
@benbjohnson benbjohnson deleted the tsm1-lint branch October 15, 2015 21:09
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