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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: remove naming series from log-like doctypes #16823

Merged
merged 1 commit into from
May 4, 2022

Conversation

ankush
Copy link
Member

@ankush ankush commented May 3, 2022

Removed naming series from:

  • webhook request log
  • access log

These are unnecessary and make operations sequential because of the naming series lock.

Example: 20+ parallel workers effectively operating sequentially because they are all waiting for lock for incrementing series counter on a log table 馃捀

2022-05-03 19 14 05

PS: If you want it on your site, it's easy to add it back from "Customize form" having this by default isn't required.

- webhook request log
- access log
@codecov
Copy link

codecov bot commented May 3, 2022

Codecov Report

Merging #16823 (aac4df4) into develop (ee40bda) will decrease coverage by 1.79%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop   #16823      +/-   ##
===========================================
- Coverage    58.09%   56.30%   -1.80%     
===========================================
  Files          765      765              
  Lines        68616    68616              
  Branches      5956     5956              
===========================================
- Hits         39863    38632    -1231     
- Misses       25154    26385    +1231     
  Partials      3599     3599              
Flag Coverage 螖
server 60.81% <酶> (-2.84%) 猬囷笍

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

@phot0n
Copy link
Contributor

phot0n commented May 3, 2022

LGTM 馃憤

PS:

  1. we could maybe even convert the engine to myiasm?
  2. maybe we can add webhook request log to log_types? - it can then be considered for autoincrement automatically for new sites 馃槵

log_types = (
"Version",
"Error Log",
"Scheduled Job Log",
"Event Sync Log",
"Event Update Log",
"Access Log",
"View Log",
"Activity Log",
"Energy Point Log",
"Notification Log",
"Email Queue",
"DocShare",
"Document Follow",
"Console Log",
)

@ankush
Copy link
Member Author

ankush commented May 4, 2022

we could maybe even convert the engine to myiasm

Patching existing data is quite painful. Also equivalent doesn't exist for PG.

@ankush ankush merged commit d1938ee into frappe:develop May 4, 2022
@ankush ankush deleted the naming_series_log branch May 4, 2022 05:24
mergify bot pushed a commit that referenced this pull request May 4, 2022
- webhook request log
- access log

(cherry picked from commit d1938ee)
ankush added a commit that referenced this pull request May 4, 2022
- webhook request log
- access log

(cherry picked from commit d1938ee)

Co-authored-by: Ankush Menat <ankush@frappe.io>
frappe-pr-bot pushed a commit that referenced this pull request May 10, 2022
# [13.29.0](v13.28.0...v13.29.0) (2022-05-10)

### Bug Fixes

* bad query if user has ' in the email address (backport [#16796](#16796)) ([#16834](#16834)) ([6cb2955](6cb2955))
* frappe.throw in case of list of errors ([#16855](#16855)) ([#16857](#16857)) ([27a9d2d](27a9d2d))
* properly validate google sheets url ([#16683](#16683)) ([f0d3a02](f0d3a02))

### Features

* disable change log ([1897af2](1897af2))

### Performance Improvements

* remove naming series from log-like doctypes ([#16823](#16823)) ([#16826](#16826)) ([bf2d6cd](bf2d6cd))
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants