gitbase: implement sql.Indexable on all tables #298

erizocosmico · 2018-06-05T14:34:11Z

Most of the contents in #279 did not survive because they relied on the index being something generic for all tables when in reality, almost every table has its own logic regarding the index storage.

This implementation differs a lot from what was initially thought in #295. The way the indexes work made it impossible to work that way.
There is also a lot of code that is inside go-git, but that logic is not exposed. If it's ever exposed we might want to refactor some of that.

To sum it up, the implementation looks like this:

Every table has its own key value iterator with its own index key, because most of them need different things stored so they can be converted to rows efficiently after getting the key from the index.
There is some common logic to find packfiles for a given repository and get from there the offsets of the objects that will be indexed.
For ref_commits, commit_trees and commit_blobs there is a common logic rowKeyValueIter and rowIndexIter, since they are stored as is. The other tables had no obvious common logic, so instead of coming up with an abstraction that might be a PITA in the future, I preferred to have some more tedious and perhaps slightly repeating implementations on the other iterators (though there are no exact identical impls).

Signed-off-by: Miguel Molina <[email protected]>

ajnavarro · 2018-06-05T16:48:58Z

Related: src-d/go-git#854

ajnavarro · 2018-06-05T16:51:47Z

commits.go

@@ -261,3 +294,107 @@ func (i *commitsByHashIter) nextList() (*object.Commit, error) {
 		return commit, nil
 	}
 }
+
+type commitIndexKey struct {


Could we have a generic ObjectIndexKey struct for all the objects that are into the packfile?

erizocosmico · 2018-06-05T18:36:34Z

Only commit and blob have the struct totally in common. Tree entries also needs the pos inside the tree and the rest of the tables are very different. I can make one common object for blobs and commits tho. On Tue, 5 Jun 2018 at 18:51, Antonio Navarro Perez <[email protected]<mailto:[email protected]>> wrote: @ajnavarro commented on this pull request.

________________________________ In commits.go<#298 (comment)>:

@@ -261,3 +294,107 @@ func (i *commitsByHashIter) nextList() (*object.Commit, error) {

return commit, nil } } + +type commitIndexKey struct { Could we have a generic ObjectIndexKey struct for all the objects that are into the packfile? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#298 (review)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ABQFFw_m3GywQHDl9yjWrDGTARu2rRCqks5t5rclgaJpZM4Ua80z>.

erizocosmico · 2018-06-07T08:14:03Z

Added the tests and some fixes, along with the unification of the index keys where it was possible. It can be reviewed now.

ajnavarro · 2018-06-08T08:46:14Z

go-git 4.4.1 has dotgit logic outside internal folder, so we can use it now updating the dependency.

erizocosmico · 2018-06-08T08:52:07Z

@ajnavarro Since this is against a feature branch and won't go straight to master, what do you think if we review and merge this one first and then in a followup PR I update the dependency and replace the manual impl with the dotgit one?

ajnavarro · 2018-06-12T07:38:56Z

packfiles.go

 		}, nil
 	}
 }

 func (i *objectIter) Close() error {
-	if i.packObjects != nil {
+	/*if i.packObjects != nil {


Signed-off-by: Miguel Molina <[email protected]>

erizocosmico · 2018-06-12T10:40:02Z

Fixed errors in travis

Signed-off-by: Miguel Molina <[email protected]>

ajnavarro · 2018-06-13T08:37:46Z

cmd/gitbase/command/server.go

+	User      string   `short:"u" long:"user" default:"root" description:"User name used for connection"`
+	Password  string   `short:"P" long:"password" default:"" description:"Password used for connection"`
+	PilosaURL string   `long:"pilosa" default:"http://localhost:10101" description:"URL to your pilosa server"`
+	IndexDir  string   `short:"i" long:"index" default:"/var/gitbase/index" description:"Directory where the gitbase indexes information will be persisted."`


Maybe we should change the default path. What do you think @jfontan ?

What path should we change it to?

Usually software does not have var directories directly in /var but in one of its subdirectories. MySQL has its database files in /var/lib/mysql. /var/lib/gitbase/index seems ok.

ajnavarro · 2018-06-13T08:38:07Z

cmd/gitbase/command/server.go

@@ -104,6 +110,18 @@ func (c *Server) buildDatabase() error {
 	c.engine.Catalog.RegisterFunctions(function.Functions)
 	logrus.Debug("registered all available functions in catalog")

+	if err := os.MkdirAll(c.IndexDir, 0755); err != nil {


Should we add some log on these new steps?

ajnavarro · 2018-06-13T08:39:52Z

files.go

+	}
+}
+
+func (i *filesKeyValueIter) Close() error {


We should call this on tests too.

ajnavarro · 2018-06-13T08:44:40Z

repositories.go

+
+	var iter sql.RowIter = &rowIndexIter{index}
+
+	if len(filters) > 0 {


Can we test this with some filter?

ajnavarro · 2018-06-13T08:52:05Z

Check codecov, appears that squash_join is not tested at all.

Signed-off-by: Miguel Molina <[email protected]>

erizocosmico · 2018-06-13T09:24:00Z

Fixed the coverage thing of the squash tables

Signed-off-by: Miguel Molina <[email protected]>

erizocosmico · 2018-06-13T09:24:56Z

Changed the default index directory

jfontan · 2018-06-13T09:41:08Z

remotes.go

+	columns, filters []sql.Expression,
+	index sql.IndexValueIter,
+) (sql.RowIter, error) {
+	span, ctx := ctx.Span("gitbase.ReferencesTable.WithProjectFiltersAndIndex")


Shouldn't this be "gitbase.RemotesTable.WithProjectFiltersAndIndex"?

Signed-off-by: Miguel Molina <[email protected]>

erizocosmico · 2018-06-13T13:33:28Z

Everything is fixed now, upgraded go-mysql-server and added engine initialization to the server command.

gitbase: implement sql.Indexable on all tables

b91f814

Signed-off-by: Miguel Molina <[email protected]>

erizocosmico requested a review from ajnavarro June 5, 2018 14:34

ajnavarro reviewed Jun 5, 2018

View reviewed changes

erizocosmico mentioned this pull request Jun 8, 2018

gitbase: use DotGit for managing pack objects #302

Closed

ajnavarro reviewed Jun 12, 2018

View reviewed changes

erizocosmico added 2 commits June 12, 2018 10:25

*: add tests for table indexes and fixes

70f7856

Signed-off-by: Miguel Molina <[email protected]>

vendor: upgrade go-git

578bba7

Signed-off-by: Miguel Molina <[email protected]>

erizocosmico added 2 commits June 12, 2018 14:34

cmd/gitbase: register pilosa index on server startup

872f4df

Signed-off-by: Miguel Molina <[email protected]>

vendor: upgrade dependencies

381994a

Signed-off-by: Miguel Molina <[email protected]>

ajnavarro reviewed Jun 13, 2018

View reviewed changes

erizocosmico added 2 commits June 13, 2018 11:04

gitbase: call Close on tests, improve coverage

b49d468

Signed-off-by: Miguel Molina <[email protected]>

gitbase: make commit_blobs and tree_entries Squashable

48d8f9f

Signed-off-by: Miguel Molina <[email protected]>

cmd/gitbase/command: change default index directory

573a162

Signed-off-by: Miguel Molina <[email protected]>

jfontan reviewed Jun 13, 2018

View reviewed changes

erizocosmico added 2 commits June 13, 2018 12:01

packfiles: use go-git DotGit to manage packfiles

1d7d8cb

Signed-off-by: Miguel Molina <[email protected]>

vendor: upgrade go-git

c37e453

Signed-off-by: Miguel Molina <[email protected]>

ajnavarro approved these changes Jun 13, 2018

View reviewed changes

erizocosmico added 2 commits June 13, 2018 15:28

gitbase: fix name of remotes table span

a0655e1

Signed-off-by: Miguel Molina <[email protected]>

vendor: upgrade go-mysql-server

6dbdc42

Signed-off-by: Miguel Molina <[email protected]>

cmd/gitbase/command: initialize engine

9ccc45c

Signed-off-by: Miguel Molina <[email protected]>

ajnavarro approved these changes Jun 13, 2018

View reviewed changes

jfontan approved these changes Jun 13, 2018

View reviewed changes

erizocosmico merged commit eb3e95b into src-d:feature/indexable Jun 13, 2018

erizocosmico deleted the feature/indexable-tables branch June 13, 2018 13:55


		var iter sql.RowIter = &rowIndexIter{index}

		if len(filters) > 0 {

gitbase: implement sql.Indexable on all tables #298

gitbase: implement sql.Indexable on all tables #298

Uh oh!

Conversation

erizocosmico commented Jun 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ajnavarro commented Jun 5, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erizocosmico commented Jun 5, 2018 via email

Uh oh!

erizocosmico commented Jun 7, 2018

Uh oh!

ajnavarro commented Jun 8, 2018

Uh oh!

erizocosmico commented Jun 8, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erizocosmico commented Jun 12, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jfontan Jun 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ajnavarro commented Jun 13, 2018

Uh oh!

erizocosmico commented Jun 13, 2018

Uh oh!

erizocosmico commented Jun 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erizocosmico commented Jun 13, 2018

Uh oh!

Uh oh!

erizocosmico commented Jun 5, 2018 •

edited

Loading

jfontan Jun 13, 2018 •

edited

Loading