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

Feature/metrics DB/SQL server #2

Merged
merged 13 commits into from
Jun 11, 2014
Merged

Feature/metrics DB/SQL server #2

merged 13 commits into from
Jun 11, 2014

Conversation

haf
Copy link
Member

@haf haf commented Jun 10, 2014

An evening hack.

To be clear: this supports any DB that https://github.com/mausch/FsSql supports.

The tests are being run with SQLite.

@karaaie - what do you think?

@mausch - The example problem comes when running Logary.DB.Tests

haf added 2 commits June 10, 2014 20:26
@haf
first draft
6c098a6
@haf
migrations supported, log lines, metrics, sqlite in tests.
36dce44 
Perhaps @mausch could lend a hand in getting the execNonQueryAsync to work
without throwing ObjectDisposedException? :)
@haf
Copy link
Member Author

haf commented Jun 10, 2014

Yes Logary.DB.Tests have it isolated.

@haf
Copy link
Member Author

haf commented Jun 10, 2014 

[D] (actors) 2014-06-10T20:46:49.3371924Z: logaryRoot/db-target pre-start Status: Shutdown ""
[D] (actors) 2014-06-10T20:46:49.3526757Z: logaryRoot/db-target started Status: Running
[D] (actors) 2014-06-10T20:46:49.3992891Z: logaryRoot/db-target pre-stop Status: Running
[D] (actors) 2014-06-10T20:46:49.3992891Z: logaryRoot/db-target pre-start Status: Shutdown ""
[D] (actors) 2014-06-10T20:46:49.3992891Z: logaryRoot/db-target started Status: Running
[D] (actors) 2014-06-10T20:46:49.3992891Z: logaryRoot/db-target stopped Status: Shutdown "actor 'logaryRoot/db-target' exited normally"
[D] (actors) 2014-06-10T20:46:50.1139255Z: logaryRoot/db-target pre-stop Status: Errored
  System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'SQLiteCommand'.
   at System.Data.SQLite.SQLiteCommand.CheckDisposed()
   at System.Data.SQLite.SQLiteCommand.ExecuteNonQuery()
   at [email protected](Unit unitVar) in C:\prg\FsSql\FsSql\AsyncExtensions.fs:line 64
   at [email protected](AsyncParams`1 args)
[D] (actors) 2014-06-10T20:46:50.1294786Z: logaryRoot/db-target stopped Status: Shutdown
  "An exception was handled
System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'SQLiteCommand'.
   at System.Data.SQLite.SQLiteCommand.CheckDisposed()
   at System.Data.SQLite.SQLiteCommand.ExecuteNonQuery()
   at [email protected](Unit unitVar) in C:\prg\FsSql\FsSql\AsyncExtensions.fs:line 64
   at [email protected](AsyncParams`1 args)"

@mausch
Copy link

mausch commented Jun 10, 2014

Sounds like mausch/FsSql#13 . Needs more investigation... concurrency bugs are always nasty :-/

@haf
Copy link
Member Author

haf commented Jun 10, 2014

I see... Do you think I should avoid async in the mean while, or will you have a look at it?

@mausch
Copy link

mausch commented Jun 10, 2014

Now that there's a test for it I might take a look, but in the meantime it's best to avoid async to be safe.

@haf
Copy link
Member Author

haf commented Jun 10, 2014

@mausch thank you. Remember it's 36dce44 that has the failing test. I'm going to try to see what happens if I use the non-async method to update this PR. :)

haf added 4 commits June 10, 2014 23:59
@haf
works with non-async method
ba1c48e
@haf
more aptly named index
d0828e9
@haf
passing tests, some TODO tests added
9c29a06 
What's left is verifying good data is saved in the fields
by completing the skipped tests.
@haf
minor formatting
84352ee
@haf haf changed the title Feature/metrics sql server Feature/metrics DB/SQL server Jun 10, 2014
haf
haf reviewed Jun 10, 2014
View reviewed changes
src/Logary.DB.Migrations/Migrations.fs
.WithColumnDescription("Arbitrary correlation id from context")
|> ignore

base.Create.Index("IX_Metrics_EpochTicksTypeLevel").OnTable(Defaults.MetricsTable)
Copy link
Member Author

Choose a reason for hiding this comment

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

@smoergaaskatten here are the indicies we talked about

Copy link

Choose a reason for hiding this comment

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

For shipping, only on EpochTicks. Searching, below is fine.

Path = server and metric path and metric name - move to individual columns possibly.

Copy link

Choose a reason for hiding this comment

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

Optimisation ~20k/s:

  • Path -> separate table, PathId
  • Type -> known ints/enum, description on col
  • CorrelationId -> CorrelationType (type of data in next col) and CorrelationId (string approx 35 no of chars?)

@haf haf merged commit df388ec into master Jun 11, 2014
haf added a commit that referenced this pull request Jun 11, 2014
@haf
Merge branch 'feature/target-db'
7b7eff0 
Closes #2 #2
@haf haf deleted the feature/metrics-sql-server branch June 11, 2014 14:56
haf pushed a commit to haf/logary that referenced this pull request Dec 11, 2015
@paavohuhtala
Merge pull request causiq#2 from haf/feature/hopac
3f2d851
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@haf @mausch @smoergaaskatten