Implement `collStats` for SQLite #3295

chilagrow · 2023-08-31T10:00:48Z

Description

Closes #3259.

Readiness checklist

I added/updated unit tests (and they pass).
I added/updated integration/compatibility tests (and they pass).
I added/updated comments and checked rendering.
I made spot refactorings.
I updated user documentation.
I ran task all, and it passed.
I ensured that PR title is good enough for the changelog.
(for maintainers only) I set Reviewers (@FerretDB/core), Milestone (Next), Labels, Project and project's Sprint fields.
I marked all done items in this checklist.

codecov · 2023-08-31T10:03:52Z

Codecov Report

Merging #3295 (d69ffef) into main (76c3b68) will increase coverage by 0.12%.
The diff coverage is 80.18%.

@@            Coverage Diff             @@
##             main    #3295      +/-   ##
==========================================
+ Coverage   75.09%   75.22%   +0.12%     
==========================================
  Files         400      400              
  Lines       23196    23374     +178     
==========================================
+ Hits        17420    17582     +162     
- Misses       4756     4765       +9     
- Partials     1020     1027       +7

Files Changed	Coverage
internal/handlers/sqlite/msg_collstats.go	`66.19%`
internal/backends/sqlite/database.go	`82.14%`
internal/handlers/sqlite/msg_aggregate.go	`86.81%`
internal/backends/sqlite/collection.go	`88.00%`
internal/backends/collection.go	`100.00%`
internal/backends/database.go	`100.00%`
internal/handlers/pg/msg_collstats.go	`100.00%`
internal/handlers/sqlite/msg_dbstats.go	`100.00%`

Flag	Coverage Δ
hana	`?`
integration	`71.80% <80.18%> (+0.15%)`	⬆️
mongodb	`5.06% <0.00%> (-0.04%)`	⬇️
pg	`61.96% <0.45%> (-0.49%)`	⬇️
shard-1	`56.60% <70.27%> (+0.88%)`	⬆️
shard-2	`55.61% <55.40%> (+1.13%)`	⬆️
shard-3	`54.35% <39.63%> (-1.60%)`	⬇️
sqlite	`55.45% <79.72%> (+0.41%)`	⬆️
unit	`24.82% <15.31%> (-0.06%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

mergify · 2023-08-31T10:25:00Z

@chilagrow this pull request has merge conflicts.

chilagrow · 2023-09-01T07:20:56Z

The numbers look fine

test> db.runCommand({collStats:"collection"})                                                                                                                                                                      
{                                                                                                                                                                                                                  
  ns: 'test.collection',                                                                                                                                                                                           
  size: Long("0"),                                                                                                                                                                                                 
  count: Long("0"),                                                                                                                                                                                                
  storageSize: Long("0"),                                                                                                                                                                                          
  nindexes: Long("0"),                                                                                                                                                                                             
  totalIndexSize: Long("0"),                                                                                                                                                                                       
  totalSize: Long("0"),                                                                                                                                                                                            
  scaleFactor: 1,                                                                                                                                                                                                  
  ok: 1                                                                                                                                                                                                            
}                                                                                                                                                                                                                  

test> db.collection.insertMany([{_id: 1}, {_id: 2}])                                                                                                                                                               
{ acknowledged: true, insertedIds: { '0': 1, '1': 2 } }                                                                                                                                                            
test> db.runCommand({collStats:"collection"})                                                                                                                                                                      
{                                                                                                                                                                                                                  
  ns: 'test.collection',                                                                                                                                                                                           
  size: Long("4096"),                                                                                                                                                                                              
  count: Long("2"),                                                                                                                                                                                                
  avgObjSize: Long("2048"),                                                                                                                                                                                        
  storageSize: Long("4096"),                                                                                                                                                                                       
  nindexes: Long("0"),                                                                                                                                                                                             
  totalIndexSize: Long("0"),                                                                                                                                                                                       
  totalSize: Long("4096"),                                                                                                                                                                                         
  scaleFactor: 1,                                                                                                                                                                                                  
  ok: 1                                                                                                                                                                                                            
}                                                                                                                                                                                                                  

test> db.collection.aggregate( [ { $collStats: {count: {}, storageStats: {}} } ] )                                                                                                                                 
[                                                                                                                                                                                                                  
  {                                                                                                                                                                                                                
    ns: 'test.collection',                                                                                                                                                                                         
    host: 'alma',                                                                                                                                                                                                  
    localTime: '2023-09-01T07:16:13Z',                                                                                                                                                                             
    storageStats: {                                                                                                                                                                                                
      size: Long("4096"),                                                                                                                                                                                          
      count: Long("2"),                                                                                                                                                                                    [0/1938]
      avgObjSize: Long("2048"),                                      
      storageSize: Long("4096"),                                     
      freeStorageSize: Long("0"),                                    
      capped: false,                                                 
      wiredTiger: {},                                                
      nindexes: Long("0"),                                           
      indexDetails: {},                                              
      indexBuilds: {},                                               
      totalIndexSize: Long("0"),                                     
      totalSize: Long("4096"),                                       
      indexSizes: {},                                                
      scaleFactor: Long("0")                                         
    },                                                               
    count: Long("2")                                                 
  }                                                                  
]

…sqlite-impt

mergify · 2023-09-01T10:40:02Z

@chilagrow this pull request has merge conflicts.

AlekSi · 2023-09-01T12:32:29Z

internal/backends/collection.go

+	defer observability.FuncCall(ctx)()
+
+	res, err := cc.c.Stats(ctx, params)
+	checkError(err, ErrorCodeCollectionDoesNotExist)


Let's do the same thing as with Database's Stats: return zero-filled struct without error. That's how we handled that "error" in the handler anyway, so why return an error at all?

For collStats command, indeed it returns zero-filled struct.
However, aggregation $collStats stage returns command error on non-existing name space, and existing compat test would fail if error isn't returned.

https://github.com/FerretDB/FerretDB/pull/3295/files#diff-b26835ea59cb934e35c55f22543373b2954d6a2f81ed7901d94d2b5e9ee0ade1R388

Then, could we do the opposite and return ErrorCodeDatabaseDoesNotExist in Database.Stats? That would make both Stats method consistent

That would work, let me do that 👍

rumyantseva

No questions about the implementation (apart from what Alexey already asked).

I have a couple of questions about how we check errors, but it's about the new backend in general, not about this particular PR, so I'll ask on Slack.

chilagrow · 2023-09-01T03:46:06Z

internal/handlers/pg/msg_collstats.go

@@ -93,7 +93,7 @@ func (h *Handler) MsgCollStats(ctx context.Context, msg *wire.OpMsg) (*wire.OpMs
 	}

 	pairs = append(pairs,
-		"storageSize", stats.SizeTotal/scale,
+		"storageSize", stats.SizeCollection/scale,


Fix

storageSize does not include index size. See totalIndexSize for index sizing.

https://www.mongodb.com/docs/manual/reference/command/collStats/#mongodb-data-collStats.storageSize

chilagrow · 2023-09-04T02:22:50Z

internal/backends/collection.go

+	defer observability.FuncCall(ctx)()
+
+	res, err := cc.c.Stats(ctx, params)
+	checkError(err, ErrorCodeCollectionDoesNotExist)


For collStats command, indeed it returns zero-filled struct.
However, aggregation $collStats stage returns command error on non-existing name space, and existing compat test would fail if error isn't returned.

https://github.com/FerretDB/FerretDB/pull/3295/files#diff-b26835ea59cb934e35c55f22543373b2954d6a2f81ed7901d94d2b5e9ee0ade1R388

mergify · 2023-09-04T04:28:56Z

@chilagrow this pull request has merge conflicts.

AlekSi

#3295 (comment) + plus there is a conflict now

chilagrow · 2023-09-04T05:49:10Z

I have a couple of questions about how we check errors, but it's about the new backend in general, not about this particular PR, so I'll ask on Slack.

@rumyantseva what are your questions?

chilagrow added 2 commits August 31, 2023 18:40

add backend interface for collection statistics

894798e

initial impl of collStats for SQLite

37ba896

chilagrow added the code/feature Some user-visible feature is not implemented yet label Aug 31, 2023

chilagrow added this to the Next milestone Aug 31, 2023

chilagrow self-assigned this Aug 31, 2023

mergify bot added the conflict PRs that have merge conflicts label Aug 31, 2023

chilagrow added 4 commits September 1, 2023 11:12

Merge branch 'main' into issue-3259-collstats-sqlite

bb37eae

add Database prefix for params and result of Stats method

8841ee5

merge

11f71ee

merge

85cf7ee

mergify bot removed the conflict PRs that have merge conflicts label Sep 1, 2023

chilagrow added 5 commits September 1, 2023 12:59

collStats indexes fail for SQLite

c5227a4

return error on non existing collection

2c7a11f

totalSize for collStats is size of index and table

b959cee

fix error message

79e435d

update test, cleanup

beb6686

tidy up

f1e8cad

chilagrow marked this pull request as ready for review September 1, 2023 07:40

chilagrow requested review from AlekSi and a team as code owners September 1, 2023 07:40

chilagrow requested review from rumyantseva, a team and noisersup September 1, 2023 07:40

chilagrow enabled auto-merge (squash) September 1, 2023 07:40

chilagrow added 3 commits September 1, 2023 17:51

update godoc

a08f4ae

Merge branch 'issue-3259-collstats-sqlite' into issue-3259-collstats-…

338fa48

…sqlite-impt

lint

8678a76

mergify bot added the conflict PRs that have merge conflicts label Sep 1, 2023

AlekSi reviewed Sep 1, 2023

View reviewed changes

rumyantseva reviewed Sep 1, 2023

View reviewed changes

merge conflict

e0485c8

mergify bot removed the conflict PRs that have merge conflicts label Sep 4, 2023

chilagrow commented Sep 4, 2023

View reviewed changes

add test

4e1b47d

chilagrow requested review from AlekSi and rumyantseva September 4, 2023 03:32

mergify bot added the conflict PRs that have merge conflicts label Sep 4, 2023

AlekSi reviewed Sep 4, 2023

View reviewed changes

chilagrow added 3 commits September 4, 2023 14:03

merge conflict

03defb2

use state provider in test

8141d40

stats on DB returns error on not found DB

1ff5889

mergify bot removed the conflict PRs that have merge conflicts label Sep 4, 2023

update test

f7f5dc5

chilagrow requested a review from AlekSi September 4, 2023 05:49

Merge branch 'main' into issue-3259-collstats-sqlite-impt

d69ffef

AlekSi approved these changes Sep 4, 2023

View reviewed changes

AlekSi disabled auto-merge September 4, 2023 10:52

AlekSi merged commit 4e8aba9 into FerretDB:main Sep 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement `collStats` for SQLite #3295

Implement `collStats` for SQLite #3295

chilagrow commented Aug 31, 2023 •

edited

Loading

codecov bot commented Aug 31, 2023 •

edited

Loading

mergify bot commented Aug 31, 2023

chilagrow commented Sep 1, 2023

mergify bot commented Sep 1, 2023

AlekSi Sep 1, 2023

chilagrow Sep 4, 2023 •

edited

Loading

AlekSi Sep 4, 2023

chilagrow Sep 4, 2023

rumyantseva left a comment

chilagrow Sep 1, 2023

chilagrow Sep 4, 2023 •

edited

Loading

mergify bot commented Sep 4, 2023

AlekSi left a comment

chilagrow commented Sep 4, 2023

Implement collStats for SQLite #3295

Implement collStats for SQLite #3295

Conversation

chilagrow commented Aug 31, 2023 • edited Loading

Description

Readiness checklist

codecov bot commented Aug 31, 2023 • edited Loading

Codecov Report

mergify bot commented Aug 31, 2023

chilagrow commented Sep 1, 2023

mergify bot commented Sep 1, 2023

AlekSi Sep 1, 2023

Choose a reason for hiding this comment

chilagrow Sep 4, 2023 • edited Loading

Choose a reason for hiding this comment

AlekSi Sep 4, 2023

Choose a reason for hiding this comment

chilagrow Sep 4, 2023

Choose a reason for hiding this comment

rumyantseva left a comment

Choose a reason for hiding this comment

chilagrow Sep 1, 2023

Choose a reason for hiding this comment

chilagrow Sep 4, 2023 • edited Loading

Choose a reason for hiding this comment

mergify bot commented Sep 4, 2023

AlekSi left a comment

Choose a reason for hiding this comment

chilagrow commented Sep 4, 2023

Implement `collStats` for SQLite #3295

Implement `collStats` for SQLite #3295

chilagrow commented Aug 31, 2023 •

edited

Loading

codecov bot commented Aug 31, 2023 •

edited

Loading

chilagrow Sep 4, 2023 •

edited

Loading

chilagrow Sep 4, 2023 •

edited

Loading