Skip to content

Commit

Permalink
refactor(handler): delay store.UserByID as much as possible
Browse files Browse the repository at this point in the history 
In internal/reader/handler/handler.go:RefreshFeed, there is a call to
store.UserByID pretty early, which is only used for
originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language)
Its only other usage is in processor.ProcessFeedEntries(store, originalFeed,
user, forceRefresh), which is pretty late in RefreshFeed, and only called if
there are new items in the feed. It makes sense to only fetch the user's
language if the error localization function is used.

Calls to `store.UserByID` take around 10% of the CPU time of RefreshFeed in my
profiling.

This commit also makes `processor.ProcessFeedEntries` take a `userID` instead
of a `user`, to make the code a bit more concise.

This should close #2984
  • Loading branch information
@jvoisin
jvoisin authored Dec 10, 2024
1 parent 02c6d14 commit 637fb85
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 19 deletions.
41 changes: 23 additions & 18 deletions internal/reader/handler/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,6 @@ func CreateFeedFromSubscriptionDiscovery(store *storage.Storage, userID int64, f
slog.String("feed_url", feedCreationRequest.FeedURL),
)

user, storeErr := store.UserByID(userID)
if storeErr != nil {
return nil, locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
}

if !store.CategoryIDExists(userID, feedCreationRequest.CategoryID) {
return nil, locale.NewLocalizedErrorWrapper(ErrCategoryNotFound, "error.category_not_found")
}
Expand Down Expand Up @@ -71,7 +66,7 @@ func CreateFeedFromSubscriptionDiscovery(store *storage.Storage, userID int64, f
subscription.WithCategoryID(feedCreationRequest.CategoryID)
subscription.CheckedNow()

processor.ProcessFeedEntries(store, subscription, user, true)
processor.ProcessFeedEntries(store, subscription, userID, true)

if storeErr := store.CreateFeed(subscription); storeErr != nil {
return nil, locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
Expand Down Expand Up @@ -105,11 +100,6 @@ func CreateFeed(store *storage.Storage, userID int64, feedCreationRequest *model
slog.String("feed_url", feedCreationRequest.FeedURL),
)

user, storeErr := store.UserByID(userID)
if storeErr != nil {
return nil, locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
}

if !store.CategoryIDExists(userID, feedCreationRequest.CategoryID) {
return nil, locale.NewLocalizedErrorWrapper(ErrCategoryNotFound, "error.category_not_found")
}
Expand Down Expand Up @@ -170,7 +160,7 @@ func CreateFeed(store *storage.Storage, userID int64, feedCreationRequest *model
subscription.WithCategoryID(feedCreationRequest.CategoryID)
subscription.CheckedNow()

processor.ProcessFeedEntries(store, subscription, user, true)
processor.ProcessFeedEntries(store, subscription, userID, true)

if storeErr := store.CreateFeed(subscription); storeErr != nil {
return nil, locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
Expand All @@ -195,11 +185,6 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool
slog.Bool("force_refresh", forceRefresh),
)

user, storeErr := store.UserByID(userID)
if storeErr != nil {
return locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
}

originalFeed, storeErr := store.FeedByID(userID, feedID)
if storeErr != nil {
return locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
Expand Down Expand Up @@ -256,13 +241,21 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool

if localizedError := responseHandler.LocalizedError(); localizedError != nil {
slog.Warn("Unable to fetch feed", slog.String("feed_url", originalFeed.FeedURL), slog.Any("error", localizedError.Error()))
user, storeErr := store.UserByID(userID)
if storeErr != nil {
return locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
}
originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language))
store.UpdateFeedError(originalFeed)
return localizedError
}

if store.AnotherFeedURLExists(userID, originalFeed.ID, responseHandler.EffectiveURL()) {
localizedError := locale.NewLocalizedErrorWrapper(ErrDuplicatedFeed, "error.duplicated_feed")
user, storeErr := store.UserByID(userID)
if storeErr != nil {
return locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
}
originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language))
store.UpdateFeedError(originalFeed)
return localizedError
Expand All @@ -289,6 +282,10 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool
if errors.Is(parseErr, parser.ErrFeedFormatNotDetected) {
localizedError = locale.NewLocalizedErrorWrapper(parseErr, "error.feed_format_not_detected", parseErr)
}
user, storeErr := store.UserByID(userID)
if storeErr != nil {
return locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
}

originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language))
store.UpdateFeedError(originalFeed)
Expand All @@ -309,13 +306,17 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool
)

originalFeed.Entries = updatedFeed.Entries
processor.ProcessFeedEntries(store, originalFeed, user, forceRefresh)
processor.ProcessFeedEntries(store, originalFeed, userID, forceRefresh)

// We don't update existing entries when the crawler is enabled (we crawl only inexisting entries). Unless it is forced to refresh
updateExistingEntries := forceRefresh || !originalFeed.Crawler
newEntries, storeErr := store.RefreshFeedEntries(originalFeed.UserID, originalFeed.ID, originalFeed.Entries, updateExistingEntries)
if storeErr != nil {
localizedError := locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
user, storeErr := store.UserByID(userID)
if storeErr != nil {
return locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
}
originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language))
store.UpdateFeedError(originalFeed)
return localizedError
Expand Down Expand Up @@ -359,6 +360,10 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool

if storeErr := store.UpdateFeed(originalFeed); storeErr != nil {
localizedError := locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
user, storeErr := store.UserByID(userID)
if storeErr != nil {
return locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
}
originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language))
store.UpdateFeedError(originalFeed)
return localizedError
Expand Down
8 changes: 7 additions & 1 deletion internal/reader/processor/processor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,15 @@ import (
var customReplaceRuleRegex = regexp.MustCompile(`rewrite\("([^"]+)"\|"([^"]+)"\)`)

// ProcessFeedEntries downloads original web page for entries and apply filters.
func ProcessFeedEntries(store *storage.Storage, feed *model.Feed, user *model.User, forceRefresh bool) {
func ProcessFeedEntries(store *storage.Storage, feed *model.Feed, userID int64, forceRefresh bool) {
var filteredEntries model.Entries

user, storeErr := store.UserByID(userID)
if storeErr != nil {
slog.Error("Database error", slog.Any("error", storeErr))
return
}

// Process older entries first
for i := len(feed.Entries) - 1; i >= 0; i-- {
entry := feed.Entries[i]
Expand Down

0 comments on commit 637fb85

Please sign in to comment.