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

add pathdb_dump tool #3707

Merged
merged 14 commits into from
Apr 16, 2020
Merged

add pathdb_dump tool #3707

merged 14 commits into from
Apr 16, 2020

Conversation

juagargi
Copy link
Contributor

@juagargi juagargi commented Apr 10, 2020

Dumps the contents of a sqlite3 path DB.


This change is Reviewable

@oncilla oncilla changed the title pathd_ dump tool. pathdb_dump tool Apr 10, 2020
@lukedirtwalker lukedirtwalker self-requested a review April 14, 2020 06:27
Copy link
Collaborator

@lukedirtwalker lukedirtwalker left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r1.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @juagargi)


go/tools/pathdb_dump/pathdb_dump.go, line 51 at r1 (raw file):

	// ?mode=ro&_journal=OFF&_mutex=no&_txlock=immediate&journal=wal&_query_only=yes
	// ?_locking=normal&immutable=true . It fails because of setting journal
	// (vendor/.../mattn/.../sqlite3.go:1480), for all journal modes)

I found the problem, I think it is what is stated here: mattn/go-sqlite3#607 (comment)

So to fix it, here remove the whole comment block and the two lines below and add:

	filename, err := dbConn(origFilename)
	if err != nil {
		errorAndQuit(err.Error())
	}

then in sqlite.go apply the patch:

diff --git a/go/lib/infra/modules/db/sqlite.go b/go/lib/infra/modules/db/sqlite.go
index 0e6b8dc..cd68353 100644
--- a/go/lib/infra/modules/db/sqlite.go
+++ b/go/lib/infra/modules/db/sqlite.go
@@ -17,6 +17,7 @@ package db
 import (
        "database/sql"
        "fmt"
+       "net/url"
 
        "github.com/scionproto/scion/go/lib/common"
        "github.com/scionproto/scion/go/lib/serrors"
@@ -42,10 +43,6 @@ func NewSqlite(path string, schema string, schemaVersion int) (*sql.DB, error) {
        }()
        // prevent weird errors. (see https://stackoverflow.com/a/35805826)
        db.SetMaxOpenConns(1)
-       if _, err = db.Exec("PRAGMA journal_mode=WAL"); err != nil {
-               return nil, common.NewBasicError("Unable to set WAL journal mode", err,
-                       "path", path)
-       }
        // Check the schema version and set up new DB if necessary.
        var existingVersion int
        err = db.QueryRow("PRAGMA user_version;").Scan(&existingVersion)
@@ -66,9 +63,18 @@ func NewSqlite(path string, schema string, schemaVersion int) (*sql.DB, error) {
 
 func open(path string) (*sql.DB, error) {
        var err error
+       u, err := url.Parse(path)
+       if err != nil {
+               return nil, common.NewBasicError("invalid connection path", err, "path", path)
+       }
+       q := u.Query()
        // Add foreign_key parameter to path to enable foreign key support.
-       uri := fmt.Sprintf("%s?_foreign_keys=1", path)
-       db, err := sql.Open("sqlite3", uri)
+       q.Set("_foreign_keys", "1")
+       // prevent weird errors. (see https://stackoverflow.com/a/35805826)
+       q.Set("_journal_mode", "WAL")
+       u.RawQuery = q.Encode()
+       path = u.String()
+       db, err := sql.Open("sqlite3", path)
        if err != nil {
                return nil, common.NewBasicError("Couldn't open SQLite database", err, "path", path)
        }

then it should work without copying the files, remove all the temp copy files.


go/tools/pathdb_dump/pathdb_dump.go, line 57 at r1 (raw file):

	db, err := sqlite.New(filename)
	if err != nil {
		errorAndQuit(err.Error())

I generally dislike this pattern as it hides what is going on. Also defers won't when using os.Exit. So instead of having this, I would use the realMain pattern similar to what we have in the servers:

func main() {
      if err := realMain(); err != nil {
          fmt.Fprintf(os.Stderr, "Error while executing: %v\n", err)
          os.Exit(1)
      }
}

and move all the code that is currently in main to realMain


go/tools/pathdb_dump/pathdb_dump.go, line 140 at r1 (raw file):

func (s segment) toString(showTimestamps bool) string {
	toRet := s.SegType.String() + "\t"

if possible we should reuse the seg.PathSegment String method.


go/tools/pathdb_dump/pathdb_dump.go, line 142 at r1 (raw file):

	toRet := s.SegType.String() + "\t"
	now := time.Now()
	updatedStr := now.Sub(s.Updated).String()

this and the one below should go into the if showTimestamps block.


go/tools/pathdb_dump/pathdb_dump.go, line 146 at r1 (raw file):

"\t: Expires in:

why having the : in front of the Expires? it kind of looks weird to me, I would remove it.


go/tools/pathdb_dump/pathdb_dump.go, line 155 at r1 (raw file):

}

// returns if this segment is < the other segment. It relies on the

lessThan returns ...


go/tools/pathdb_dump/pathdb_dump.go, line 156 at r1 (raw file):

// returns if this segment is < the other segment. It relies on the
// short circuit of the OR op. E.g. (for two dimensions):

I think it would make more sense to specify in words after which criterias this will order. not so much on what internal programming techniques it uses.


go/tools/pathdb_dump/pathdb_dump.go, line 170 at r1 (raw file):

	}
	// reversed Type comparison so core < down < up
	return s.SegType > o.SegType || (s.SegType == o.SegType &&

instead of short circuiting and creating something that is very hard to read, I really like Go's switch statements for comparisons:

	switch {
	case s.SegType != o.SegType:
		// reversed Type comparison so core < down < up
		return s.SegType > o.SegType
	case len(s.interfaces) != len(o.interfaces):
		return len(s.interfaces) < len(o.interfaces)
	default:
		return segsLessThan(s, o)
	}

go/tools/pathdb_dump/pathdb_dump.go, line 195 at r1 (raw file):

// returns the name of the created file
func copyDBToTemp(filename string) string {

remove


go/tools/pathdb_dump/pathdb_dump.go, line 227 at r1 (raw file):

}

func removeAllDir(dirName string) {

remove

Copy link
Collaborator

@lukedirtwalker lukedirtwalker left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @juagargi)


go/tools/pathdb_dump/pathdb_dump.go, line 140 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

if possible we should reuse the seg.PathSegment String method.

In any case I think it makes sense to print the segment's logging ID (see seg.PathSegment.GetLoggingID

Copy link
Contributor Author

@juagargi juagargi left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @lukedirtwalker)


go/tools/pathdb_dump/pathdb_dump.go, line 51 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

I found the problem, I think it is what is stated here: mattn/go-sqlite3#607 (comment)

So to fix it, here remove the whole comment block and the two lines below and add:

	filename, err := dbConn(origFilename)
	if err != nil {
		errorAndQuit(err.Error())
	}

then in sqlite.go apply the patch:

diff --git a/go/lib/infra/modules/db/sqlite.go b/go/lib/infra/modules/db/sqlite.go
index 0e6b8dc..cd68353 100644
--- a/go/lib/infra/modules/db/sqlite.go
+++ b/go/lib/infra/modules/db/sqlite.go
@@ -17,6 +17,7 @@ package db
 import (
        "database/sql"
        "fmt"
+       "net/url"
 
        "github.com/scionproto/scion/go/lib/common"
        "github.com/scionproto/scion/go/lib/serrors"
@@ -42,10 +43,6 @@ func NewSqlite(path string, schema string, schemaVersion int) (*sql.DB, error) {
        }()
        // prevent weird errors. (see https://stackoverflow.com/a/35805826)
        db.SetMaxOpenConns(1)
-       if _, err = db.Exec("PRAGMA journal_mode=WAL"); err != nil {
-               return nil, common.NewBasicError("Unable to set WAL journal mode", err,
-                       "path", path)
-       }
        // Check the schema version and set up new DB if necessary.
        var existingVersion int
        err = db.QueryRow("PRAGMA user_version;").Scan(&existingVersion)
@@ -66,9 +63,18 @@ func NewSqlite(path string, schema string, schemaVersion int) (*sql.DB, error) {
 
 func open(path string) (*sql.DB, error) {
        var err error
+       u, err := url.Parse(path)
+       if err != nil {
+               return nil, common.NewBasicError("invalid connection path", err, "path", path)
+       }
+       q := u.Query()
        // Add foreign_key parameter to path to enable foreign key support.
-       uri := fmt.Sprintf("%s?_foreign_keys=1", path)
-       db, err := sql.Open("sqlite3", uri)
+       q.Set("_foreign_keys", "1")
+       // prevent weird errors. (see https://stackoverflow.com/a/35805826)
+       q.Set("_journal_mode", "WAL")
+       u.RawQuery = q.Encode()
+       path = u.String()
+       db, err := sql.Open("sqlite3", path)
        if err != nil {
                return nil, common.NewBasicError("Couldn't open SQLite database", err, "path", path)
        }

then it should work without copying the files, remove all the temp copy files.

Done. Please check commit b1ae4ea7 as I had to work around paths not being URLs (e.g. :memory:). I could just remove all the url treatment also.


go/tools/pathdb_dump/pathdb_dump.go, line 57 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

I generally dislike this pattern as it hides what is going on. Also defers won't when using os.Exit. So instead of having this, I would use the realMain pattern similar to what we have in the servers:

func main() {
      if err := realMain(); err != nil {
          fmt.Fprintf(os.Stderr, "Error while executing: %v\n", err)
          os.Exit(1)
      }
}

and move all the code that is currently in main to realMain

Done. Thanks for noticing this, I didn't know about defer not working when os.Exit


go/tools/pathdb_dump/pathdb_dump.go, line 140 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

In any case I think it makes sense to print the segment's logging ID (see seg.PathSegment.GetLoggingID

I wanted to use it, but for the time fields to be together I'd have to be able to reorganize the returned string from PathSegment.String. It's doable if we don't care for those fields to be together.


go/tools/pathdb_dump/pathdb_dump.go, line 142 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

this and the one below should go into the if showTimestamps block.

Done.


go/tools/pathdb_dump/pathdb_dump.go, line 146 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…
"\t: Expires in:

why having the : in front of the Expires? it kind of looks weird to me, I would remove it.

Done.


go/tools/pathdb_dump/pathdb_dump.go, line 155 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

lessThan returns ...

Done.


go/tools/pathdb_dump/pathdb_dump.go, line 156 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

I think it would make more sense to specify in words after which criterias this will order. not so much on what internal programming techniques it uses.

Done.


go/tools/pathdb_dump/pathdb_dump.go, line 170 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

instead of short circuiting and creating something that is very hard to read, I really like Go's switch statements for comparisons:

	switch {
	case s.SegType != o.SegType:
		// reversed Type comparison so core < down < up
		return s.SegType > o.SegType
	case len(s.interfaces) != len(o.interfaces):
		return len(s.interfaces) < len(o.interfaces)
	default:
		return segsLessThan(s, o)
	}

Done.


go/tools/pathdb_dump/pathdb_dump.go, line 195 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

remove

Done.


go/tools/pathdb_dump/pathdb_dump.go, line 227 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

remove

Done.

Copy link
Collaborator

@lukedirtwalker lukedirtwalker left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r2.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @juagargi)


go/lib/infra/modules/db/sqlite.go, line 69 at r2 (raw file):

	if err != nil {
		// may not be a URL e.g. :memory: Just go on
		path = fmt.Sprintf("%s?_foreign_keys=1&_journal_mode=WAL", path)

I assume you did this to fix tests? I think the cleaner solution would be to just use file::memory: as a connection string in test, as this would be a valid URL again.


go/tools/pathdb_dump/pathdb_dump.go, line 95 at r2 (raw file):

		return ""
	}
	strs := []string{fmt.Sprintf("%s %d", ifs[0].IA, ifs[0].ifNum)}

I played around with using %4d everywhere instead of %d in this function. I makes the output a bit more structured (all IA are below each other) but I'll leave it up to you if you prefer it. (non-blocking)


go/tools/pathdb_dump/pathdb_dump.go, line 143 at r2 (raw file):

"%s\t%s\t%s

I think using spaces is cleaner, no need for so much distance here: "%s %4s %s"


go/tools/pathdb_dump/pathdb_dump.go, line 169 at r2 (raw file):

	case s.SegType != o.SegType:
		// reversed Type comparison so core < down < up
		return s.SegType > o.SegType

What do you think of adding 3 extra cases, that makes the output sorted by start & endpoint. IMO this makes it much easier to look stuff up in the output as a human. e.g. are there paths between x and y becomes much easier to answer.

	case len(s.interfaces) == 0 || len(o.interfaces) == 0:
		return len(s.interfaces) < len(o.interfaces)
	case s.interfaces[0].IA.IAInt() != o.interfaces[0].IA.IAInt():
		return s.interfaces[0].IA.IAInt() < o.interfaces[0].IA.IAInt()
	case s.interfaces[len(s.interfaces)-1].IA.IAInt() !=
		o.interfaces[len(o.interfaces)-1].IA.IAInt():
		return s.interfaces[len(s.interfaces)-1].IA.IAInt() <
			o.interfaces[len(o.interfaces)-1].IA.IAInt()

example output of large topo:

6dac5763fe2d    core    1-ff00:0:120 6>1 1-ff00:0:110   Updated: 9.944288859s   Expires in: 5h59m16.611876112s
30e6c0ebbd4a    core    1-ff00:0:120 1>1005 1-ff00:0:130 1004>2 1-ff00:0:110    Updated: 4.906799811s   Expires in: 5h59m16.611849191s
6d918e434a60    core    1-ff00:0:120 2>501 2-ff00:0:220 503>450 2-ff00:0:210 453>3 1-ff00:0:110 Updated: 4.90294642s    Expires in: 5h59m16.611834848s
042e20c0710e    core    1-ff00:0:120 3>502 2-ff00:0:220 503>450 2-ff00:0:210 453>3 1-ff00:0:110 Updated: 4.912804752s   Expires in: 5h59m16.611812226s
4bf1be5a554c    core    1-ff00:0:130 1004>2 1-ff00:0:110        Updated: 4.906168507s   Expires in: 5h59m22.611800056s
272fcbaae0c6    core    1-ff00:0:130 1005>1 1-ff00:0:120 6>1 1-ff00:0:110       Updated: 4.908577426s   Expires in: 5h59m17.611787909s
2aee18814a37    core    1-ff00:0:130 1005>1 1-ff00:0:120 2>501 2-ff00:0:220 503>450 2-ff00:0:210 453>3 1-ff00:0:110     Updated: 4.907670411s   Expires in: 5h59m11.611772736s
14481b05649a    core    1-ff00:0:130 1005>1 1-ff00:0:120 3>502 2-ff00:0:220 503>450 2-ff00:0:210 453>3 1-ff00:0:110     Updated: 4.90948722s    Expires in: 5h59m11.611755885s
e7f5cdaf45cb    core    2-ff00:0:210 453>3 1-ff00:0:110 Updated: 4.891701935s   Expires in: 5h59m18.61174598s
cfca80efe1ac    core    2-ff00:0:210 450>503 2-ff00:0:220 501>2 1-ff00:0:120 6>1 1-ff00:0:110   Updated: 4.893399771s   Expires in: 5h59m13.61173187s
e6fc51c45d77    core    2-ff00:0:210 450>503 2-ff00:0:220 502>3 1-ff00:0:120 6>1 1-ff00:0:110   Updated: 4.892563479s   Expires in: 5h59m13.611715407s
077b9c600d14    core    2-ff00:0:210 450>503 2-ff00:0:220 501>2 1-ff00:0:120 1>1005 1-ff00:0:130 1004>2 1-ff00:0:110    Updated: 4.910654281s   Expires in: 5h59m7.611698958s
b174f052784e    core    2-ff00:0:210 450>503 2-ff00:0:220 502>3 1-ff00:0:120 1>1005 1-ff00:0:130 1004>2 1-ff00:0:110    Updated: 4.8944782s     Expires in: 5h59m7.61168261s
7ac7c183fea0    core    2-ff00:0:220 501>2 1-ff00:0:120 6>1 1-ff00:0:110        Updated: 4.89608677s    Expires in: 5h59m16.611670452s
69d245992129    core    2-ff00:0:220 502>3 1-ff00:0:120 6>1 1-ff00:0:110        Updated: 4.903961515s   Expires in: 5h59m16.611656028s
7b7dd177a628    core    2-ff00:0:220 503>450 2-ff00:0:210 453>3 1-ff00:0:110    Updated: 4.895423058s   Expires in: 5h59m16.611643548s
4cb25543fa71    core    2-ff00:0:220 501>2 1-ff00:0:120 1>1005 1-ff00:0:130 1004>2 1-ff00:0:110 Updated: 4.904734305s   Expires in: 5h59m11.611626141s
4c3226de4cee    core    2-ff00:0:220 502>3 1-ff00:0:120 1>1005 1-ff00:0:130 1004>2 1-ff00:0:110 Updated: 4.905563437s   Expires in: 5h59m11.611613885s
34c27c3ad72f    down    1-ff00:0:120 5>104 1-ff00:0:111 Updated: 2.391074691s   Expires in: 5h59m22.611604616s
764177ae1c8c    down    1-ff00:0:120 5>104 1-ff00:0:111 103>4094 1-ff00:0:112   Updated: 401.756263ms   Expires in: 5h59m22.611591567s
41187899827c    down    1-ff00:0:120 4>3 1-ff00:0:121   Updated: 372.426066ms   Expires in: 5h59m22.611580115s
6a983522a3a4    down    1-ff00:0:120 4>3 1-ff00:0:121 2>2 1-ff00:0:122  Updated: 4.35364239s    Expires in: 5h59m16.611567321s
0679368585a5    down    1-ff00:0:130 1002>105 1-ff00:0:111      Updated: 2.548739733s   Expires in: 5h59m22.611557093s
60ab8deb3437    down    1-ff00:0:130 1003>4095 1-ff00:0:112     Updated: 536.411124ms   Expires in: 5h59m22.611545783s
637a2650ac23    down    1-ff00:0:130 1002>105 1-ff00:0:111 103>4094 1-ff00:0:112        Updated: 519.642809ms   Expires in: 5h59m22.611534095s
0d4cfb28c2e0    down    1-ff00:0:130 1001>4079 1-ff00:0:131     Updated: 557.652188ms   Expires in: 5h59m22.611522952s
1bd70b7233bb    down    1-ff00:0:130 1001>4079 1-ff00:0:131 4078>2 1-ff00:0:132 Updated: 4.558660756s   Expires in: 5h59m17.611509866s
2f777056982f    down    1-ff00:0:130 1001>4079 1-ff00:0:131 4078>2 1-ff00:0:132 1>2 1-ff00:0:133        Updated: 3.557888648s   Expires in: 5h59m17.611495387s

Copy link
Collaborator

@lukedirtwalker lukedirtwalker left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @juagargi)


go/tools/pathdb_dump/pathdb_dump.go, line 35 at r2 (raw file):

)

func main() {

please also add a version flag,:

	if *version {
		fmt.Print(env.VersionInfo())
		os.Exit(0)
	}

Copy link
Collaborator

@lukedirtwalker lukedirtwalker left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @juagargi)


go/tools/pathdb_dump/pathdb_dump.go, line 35 at r2 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

please also add a version flag,:

	if *version {
		fmt.Print(env.VersionInfo())
		os.Exit(0)
	}
	version    = flag.Bool("version", false, "Output version information and exit.")

Copy link
Contributor Author

@juagargi juagargi left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 4 files at r1.
Reviewable status: 2 of 11 files reviewed, 4 unresolved discussions (waiting on @lukedirtwalker)


go/lib/infra/modules/db/sqlite.go, line 69 at r2 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

I assume you did this to fix tests? I think the cleaner solution would be to just use file::memory: as a connection string in test, as this would be a valid URL again.

Done.


go/tools/pathdb_dump/pathdb_dump.go, line 35 at r2 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…
	version    = flag.Bool("version", false, "Output version information and exit.")

Done.


go/tools/pathdb_dump/pathdb_dump.go, line 95 at r2 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

I played around with using %4d everywhere instead of %d in this function. I makes the output a bit more structured (all IA are below each other) but I'll leave it up to you if you prefer it. (non-blocking)

Done.


go/tools/pathdb_dump/pathdb_dump.go, line 143 at r2 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…
"%s\t%s\t%s

I think using spaces is cleaner, no need for so much distance here: "%s %4s %s"

Done. Notice that I added a vertical bar "|" to separate the time stamp information, when shown.


go/tools/pathdb_dump/pathdb_dump.go, line 169 at r2 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

What do you think of adding 3 extra cases, that makes the output sorted by start & endpoint. IMO this makes it much easier to look stuff up in the output as a human. e.g. are there paths between x and y becomes much easier to answer.

	case len(s.interfaces) == 0 || len(o.interfaces) == 0:
		return len(s.interfaces) < len(o.interfaces)
	case s.interfaces[0].IA.IAInt() != o.interfaces[0].IA.IAInt():
		return s.interfaces[0].IA.IAInt() < o.interfaces[0].IA.IAInt()
	case s.interfaces[len(s.interfaces)-1].IA.IAInt() !=
		o.interfaces[len(o.interfaces)-1].IA.IAInt():
		return s.interfaces[len(s.interfaces)-1].IA.IAInt() <
			o.interfaces[len(o.interfaces)-1].IA.IAInt()

example output of large topo:

6dac5763fe2d    core    1-ff00:0:120 6>1 1-ff00:0:110   Updated: 9.944288859s   Expires in: 5h59m16.611876112s
30e6c0ebbd4a    core    1-ff00:0:120 1>1005 1-ff00:0:130 1004>2 1-ff00:0:110    Updated: 4.906799811s   Expires in: 5h59m16.611849191s
6d918e434a60    core    1-ff00:0:120 2>501 2-ff00:0:220 503>450 2-ff00:0:210 453>3 1-ff00:0:110 Updated: 4.90294642s    Expires in: 5h59m16.611834848s
042e20c0710e    core    1-ff00:0:120 3>502 2-ff00:0:220 503>450 2-ff00:0:210 453>3 1-ff00:0:110 Updated: 4.912804752s   Expires in: 5h59m16.611812226s
4bf1be5a554c    core    1-ff00:0:130 1004>2 1-ff00:0:110        Updated: 4.906168507s   Expires in: 5h59m22.611800056s
272fcbaae0c6    core    1-ff00:0:130 1005>1 1-ff00:0:120 6>1 1-ff00:0:110       Updated: 4.908577426s   Expires in: 5h59m17.611787909s
2aee18814a37    core    1-ff00:0:130 1005>1 1-ff00:0:120 2>501 2-ff00:0:220 503>450 2-ff00:0:210 453>3 1-ff00:0:110     Updated: 4.907670411s   Expires in: 5h59m11.611772736s
14481b05649a    core    1-ff00:0:130 1005>1 1-ff00:0:120 3>502 2-ff00:0:220 503>450 2-ff00:0:210 453>3 1-ff00:0:110     Updated: 4.90948722s    Expires in: 5h59m11.611755885s
e7f5cdaf45cb    core    2-ff00:0:210 453>3 1-ff00:0:110 Updated: 4.891701935s   Expires in: 5h59m18.61174598s
cfca80efe1ac    core    2-ff00:0:210 450>503 2-ff00:0:220 501>2 1-ff00:0:120 6>1 1-ff00:0:110   Updated: 4.893399771s   Expires in: 5h59m13.61173187s
e6fc51c45d77    core    2-ff00:0:210 450>503 2-ff00:0:220 502>3 1-ff00:0:120 6>1 1-ff00:0:110   Updated: 4.892563479s   Expires in: 5h59m13.611715407s
077b9c600d14    core    2-ff00:0:210 450>503 2-ff00:0:220 501>2 1-ff00:0:120 1>1005 1-ff00:0:130 1004>2 1-ff00:0:110    Updated: 4.910654281s   Expires in: 5h59m7.611698958s
b174f052784e    core    2-ff00:0:210 450>503 2-ff00:0:220 502>3 1-ff00:0:120 1>1005 1-ff00:0:130 1004>2 1-ff00:0:110    Updated: 4.8944782s     Expires in: 5h59m7.61168261s
7ac7c183fea0    core    2-ff00:0:220 501>2 1-ff00:0:120 6>1 1-ff00:0:110        Updated: 4.89608677s    Expires in: 5h59m16.611670452s
69d245992129    core    2-ff00:0:220 502>3 1-ff00:0:120 6>1 1-ff00:0:110        Updated: 4.903961515s   Expires in: 5h59m16.611656028s
7b7dd177a628    core    2-ff00:0:220 503>450 2-ff00:0:210 453>3 1-ff00:0:110    Updated: 4.895423058s   Expires in: 5h59m16.611643548s
4cb25543fa71    core    2-ff00:0:220 501>2 1-ff00:0:120 1>1005 1-ff00:0:130 1004>2 1-ff00:0:110 Updated: 4.904734305s   Expires in: 5h59m11.611626141s
4c3226de4cee    core    2-ff00:0:220 502>3 1-ff00:0:120 1>1005 1-ff00:0:130 1004>2 1-ff00:0:110 Updated: 4.905563437s   Expires in: 5h59m11.611613885s
34c27c3ad72f    down    1-ff00:0:120 5>104 1-ff00:0:111 Updated: 2.391074691s   Expires in: 5h59m22.611604616s
764177ae1c8c    down    1-ff00:0:120 5>104 1-ff00:0:111 103>4094 1-ff00:0:112   Updated: 401.756263ms   Expires in: 5h59m22.611591567s
41187899827c    down    1-ff00:0:120 4>3 1-ff00:0:121   Updated: 372.426066ms   Expires in: 5h59m22.611580115s
6a983522a3a4    down    1-ff00:0:120 4>3 1-ff00:0:121 2>2 1-ff00:0:122  Updated: 4.35364239s    Expires in: 5h59m16.611567321s
0679368585a5    down    1-ff00:0:130 1002>105 1-ff00:0:111      Updated: 2.548739733s   Expires in: 5h59m22.611557093s
60ab8deb3437    down    1-ff00:0:130 1003>4095 1-ff00:0:112     Updated: 536.411124ms   Expires in: 5h59m22.611545783s
637a2650ac23    down    1-ff00:0:130 1002>105 1-ff00:0:111 103>4094 1-ff00:0:112        Updated: 519.642809ms   Expires in: 5h59m22.611534095s
0d4cfb28c2e0    down    1-ff00:0:130 1001>4079 1-ff00:0:131     Updated: 557.652188ms   Expires in: 5h59m22.611522952s
1bd70b7233bb    down    1-ff00:0:130 1001>4079 1-ff00:0:131 4078>2 1-ff00:0:132 Updated: 4.558660756s   Expires in: 5h59m17.611509866s
2f777056982f    down    1-ff00:0:130 1001>4079 1-ff00:0:131 4078>2 1-ff00:0:132 1>2 1-ff00:0:133        Updated: 3.557888648s   Expires in: 5h59m17.611495387s

Done.

Copy link
Collaborator

@lukedirtwalker lukedirtwalker left a comment

Choose a reason for hiding this comment

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

Reviewed 9 of 9 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @juagargi)


go/tools/pathdb_dump/pathdb_dump.go, line 170 at r2 (raw file):

		// reversed Type comparison so core < down < up
		return s.SegType > o.SegType
	case len(s.interfaces) != len(o.interfaces):

this case len(s.interfaces) != len(o.interfaces): must still be there before the default case. otherwise we might panick inside segsLessThan

Copy link
Contributor Author

@juagargi juagargi left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lukedirtwalker)


go/tools/pathdb_dump/pathdb_dump.go, line 170 at r2 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

this case len(s.interfaces) != len(o.interfaces): must still be there before the default case. otherwise we might panick inside segsLessThan

Done.

Copy link
Collaborator

@lukedirtwalker lukedirtwalker left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r4.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Collaborator

@lukedirtwalker lukedirtwalker left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@lukedirtwalker lukedirtwalker changed the title pathdb_dump tool add pathdb_dump tool Apr 16, 2020
@lukedirtwalker lukedirtwalker merged commit 7b10623 into scionproto:master Apr 16, 2020
oncilla added a commit that referenced this pull request Apr 21, 2020
Cobra has shown to be a very useful framework for writing CLI applications.
It makes it easy to write applications with many sub-commands and high configurability.

We currently have many binaries in the code base, that do not really warrant to be their own application. With the approach that we put most application logic in `go/pkg/...`, we can easily create standalone binaries (when needed/desired) and also bundle them in an all-in-one scion command.

This PR is a exploration how this could look like.
At the base is the `scion` command that bundles all the other commands.

For this exploration, I moved the showpaths logic to `go/pkgs/showpaths` such that it can be imported as a library. The old `showpaths` binary is not affected by the change, and if we continue to support the standalone binary, this will be easy to do.

Under `go/scion` the all-in-on binary is set up. Currently, it only has 3 commands:
- `completion` : generate auto-completion
- `version`: display the scion version
- `showpaths`: run the showpaths application

The nice thing is, that we gradually can add sub-commands that make sense to be bundled.
E.g., I envision `scion echo`, `scion traceroute`, `scion pingpong`, and the soon to be added `scion pathdbdump` (#3707) tools to be part of `scion` eventually.
matzf pushed a commit to netsec-ethz/scion that referenced this pull request Apr 27, 2020
Dumps the contents of a sqlite3 path DB.
matzf added a commit to netsec-ethz/scion-apps that referenced this pull request Apr 27, 2020
The pathdb_dump tool was added to scionproto/scion in
scionproto/scion#3707.
matzf added a commit to netsec-ethz/scion-apps that referenced this pull request Apr 27, 2020
The pathdb_dump tool was added to scionproto/scion in
scionproto/scion#3707.
@juagargi juagargi deleted the pathdb_dump branch July 21, 2020 07:39
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.

2 participants