Skip to content

feature: add noreturn opt for some DML operations #356

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

Merged

Conversation

GRISHNOV
Copy link
Contributor

@GRISHNOV GRISHNOV commented Mar 27, 2023

This patch introduces noreturn opt for DML operarions:

  • insert, insert_object, insert_many, insert_object_many
  • replace, replace_object, replace_many, insert_object_many
  • upsert, upsert_object, upsert_many, upsert_object_many
  • update
  • delete

The opt allows to suppress returning successfully processed tuple(s).

  • Tests
  • Changelog
  • Documentation
tarantool> crud.insert_many('customers', {
    {10, box.NULL, 'Elizabeth', 23},
    {20, box.NULL, 'Anastasia', 22},
  })
---
- rows:
  - [10, 569, 'Elizabeth', 23]
  - [20, 826, 'Anastasia', 22]
  metadata: [{'name': 'id', 'type': 'unsigned'}, {'name': 'bucket_id', 'type': 'unsigned'},
    {'name': 'name', 'type': 'string'}, {'name': 'age', 'type': 'number'}]
- null
...


tarantool> crud.insert_many('customers', {
    {30, box.NULL, 'Elizabeth', 23},
    {40, box.NULL, 'Anastasia', 22},
  }, { noreturn = true })
---
- null
- null
...


tarantool> crud.replace_object_many('customers', {}, {
    stop_on_error = true,
    rollback_on_error  = true,
    noreturn=true,
})
---
- null
- []
...


tarantool> crud.delete('customers', 1, {noreturn=true})
---
- null
- null
...
==== BATCH COMPARISON PERFORMANCE REPORT ====


== SUCCESS REQUESTS ==
(The higher the better)

|                        |         1 |           10 |          100 |      1000 |     10000 |
| ---------------------- |---------- | ------------ | ------------ | --------- | --------- |
| insert_many (noreturn) |    126625 |        88188 |        30930 |      4441 |       548 |
|            insert_many |    129512 |        84003 |        27593 |      4046 |       491 |


== SUCCESS REQUESTS PER SECOND ==
(The higher the better)

|                        |         1 |           10 |          100 |      1000 |     10000 |
| ---------------------- |---------- | ------------ | ------------ | --------- | --------- |
| insert_many (noreturn) |   4220.82 |      2939.59 |      1030.98 |    148.02 |     18.26 |
|            insert_many |   4317.06 |      2800.09 |       919.75 |    134.82 |     16.36 |


== AVERAGE CALL TIME ==
(The lower the better)

|                        |         1 |           10 |          100 |      1000 |     10000 |
| ---------------------- | --------- | ------------ | ------------ | --------- | --------- |
| insert_many (noreturn) |  0.236 ms |     0.339 ms |     0.969 ms |  6.751 ms | 54.766 ms |
|            insert_many |  0.231 ms |     0.356 ms |     1.086 ms |  7.414 ms | 61.131 ms |


== MAX CALL TIME ==
(The lower the better)

|                        |          1 |           10 |          100 |      1000 |     10000 |
| ---------------------- | ---------- | ------------ | ------------ |---------- |---------- |
| insert_many (noreturn) |  21.071 ms |    25.501 ms |    23.810 ms | 38.389 ms | 79.063 ms |
|            insert_many |  21.615 ms |    25.376 ms |    27.647 ms | 45.744 ms | 88.411 ms |

Closes #267

@DifferentialOrange
Copy link
Member

tarantool> crud.insert_many('customers', {
    {30, box.NULL, 'Elizabeth', 23},
    {40, box.NULL, 'Anastasia', 22},
  }, { noreturn = true })
---
- null
- null
...

Receiving nil, nil is rather confusing. Let's do true, nil like in truncate.

@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-267-noreturn-opt-for-DML-operations branch from 9887bc1 to 4b100cc Compare March 28, 2023 08:05
@GRISHNOV
Copy link
Contributor Author

Receiving nil, nil is rather confusing. Let's do true, nil like in truncate.

Done, PR description updated

@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-267-noreturn-opt-for-DML-operations branch 5 times, most recently from 91c5357 to dcba2f2 Compare March 29, 2023 11:31
@GRISHNOV
Copy link
Contributor Author

Receiving nil, nil is rather confusing. Let's do true, nil like in truncate.

Done, PR description updated

it was decided to return to the idea:

  • nil, nil for a completely successful batch
  • nil, err if at least one tuple failed

@GRISHNOV GRISHNOV marked this pull request as ready for review March 29, 2023 11:40
@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-267-noreturn-opt-for-DML-operations branch from dcba2f2 to dc75380 Compare March 29, 2023 13:43
Copy link
Member

@DifferentialOrange DifferentialOrange left a comment

Choose a reason for hiding this comment

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

Let's change performance test cases to use noreturn

@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-267-noreturn-opt-for-DML-operations branch from dc75380 to 492c870 Compare March 31, 2023 10:34
@GRISHNOV
Copy link
Contributor Author

Let's change performance test cases to use noreturn

Added a separate performance test for insert_many (noreturn):

==== STATISTICS PERFORMANCE REPORT ====


== SUCCESS REQUESTS ==
(The higher the better)

|                                   | crud (without stats wrapper) | crud (stats disabled) | crud (local stats) | crud (metrics stats, no quantiles) | crud (metrics stats, with quantiles) |
| --------------------------------- | ---------------------------- | --------------------- | ------------------ | ---------------------------------- | ------------------------------------ |
|                      select by pk |                              |                       |                    |                                    |                                      |
|        select gt by pk (limit 10) |                              |                       |                    |                                    |                                      |
|        pairs gt by pk (limit 100) |                              |                       |                    |                                    |                                      |
| select eq by secondary (limit 10) |                              |                       |                    |                                    |                                      |
|   select eq by sharding secondary |                              |                       |                    |                                    |                                      |
|                            insert |                       516804 |                551412 |             522171 |                             495607 |                               457784 |
|                       insert_many |                       126982 |                473683 |             472113 |                             420794 |                               418445 |
|            insert_many (noreturn) |                       130216 |                511051 |             499745 |                             478886 |                               430803 |

...
...
...


==== BATCH COMPARISON PERFORMANCE REPORT ====


== SUCCESS REQUESTS ==
(The higher the better)

|                                   |            1 |           10 |          100 |         1000 |        10000 |
| --------------------------------- | ------------ | ------------ | ------------ | ------------ | ------------ |
|                            insert |       127672 |        17440 |         1924 |          190 |           19 |
|                       insert_many |       118009 |        78974 |        28482 |         3978 |          484 |
|            insert_many (noreturn) |       130131 |        85822 |        30740 |         4394 |          559 |

...
...
...

Copy link
Member

@DifferentialOrange DifferentialOrange left a comment

Choose a reason for hiding this comment

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

Please, rebase on master to verify that dev_checks are fine

This patch introduces `noreturn` opt for DML operarions:
`insert`, `insert_object`, `insert_many`, `insert_object_many`,
`replace`, `replace_object`, `replace_many`, `insert_object_many`,
`upsert`, `upsert_object`, `upsert_many`, `upsert_object_many`,
`update`, `delete`.
The opt allows to suppress returning successfully processed tuple(s).

Closes #267
@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-267-noreturn-opt-for-DML-operations branch from 492c870 to af0ce90 Compare March 31, 2023 19:21
@GRISHNOV
Copy link
Contributor Author

Please, rebase on master to verify that dev_checks are fine

Done

@DifferentialOrange DifferentialOrange merged commit 781ef16 into master Apr 3, 2023
@DifferentialOrange DifferentialOrange deleted the igrishnov/gh-267-noreturn-opt-for-DML-operations branch April 3, 2023 07:24
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.

Add new flag "return nothing" in options for DML operations
2 participants