Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

Cassandra metadata cache #3097

Merged
merged 5 commits into from
Jul 8, 2022
Merged

Conversation

aravindanr
Copy link
Collaborator

Pull Request type

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes (Please run ./gradlew generateLock saveLock to refresh dependencies)
  • WHOSUSING.md
  • Other (please describe):

NOTE: Please remember to run ./gradlew spotlessApply to fix any format violations.

Changes in this PR

Caches for CassandraMetadataDAO and CassandraEventHandlerDAO are extracted into decorators, CacheableMetadataDAO and CacheableEventHandlerDAO respectively

Issue: Including cache within the DAO makes it impossible to opt-out of it.

Alternatives considered

None

@aravindanr aravindanr requested a review from jxu-nflx July 8, 2022 17:35

@Override
public List<EventHandler> getAllEventHandlers() {
return cassandraEventHandlerDAO.getAllEventHandlers();
Copy link
Contributor

Choose a reason for hiding this comment

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

Not reading from cache first?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done


@Override
public List<EventHandler> getEventHandlersForEvent(String event, boolean activeOnly) {
return cassandraEventHandlerDAO.getEventHandlersForEvent(event, activeOnly);
Copy link
Contributor

Choose a reason for hiding this comment

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

read from cache first?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

return cassandraMetadataDAO.getAllWorkflowDefs();
}

private void refreshTaskDefsCache() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does @trace create metrics for this method call as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

If not, we can add a metrics to the getAllTaskDefsFromDB method of CassdrandMetaDataDao

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Does @trace create metrics for this method call as well?

No. @trace does not work on private methods. The plan is to use Spring Cacheable in the internal implementation which allows greater flexibility in configuration and also exposes metrics.

LOGGER.error("refresh TaskDefs failed ", e);
}
}

private TaskDef getTaskDefFromDB(String name) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add taskdefname tag to the metric?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added

@aravindanr aravindanr requested a review from jxu-nflx July 8, 2022 18:35
@aravindanr aravindanr added the type: maintenance Refactoring label Jul 8, 2022
@aravindanr aravindanr merged commit 7882c61 into main Jul 8, 2022
@aravindanr aravindanr deleted the cassandra_metadata_cache_optimization branch July 8, 2022 18:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants