Skip to content

Commit

Permalink
Remove read only from most query peristence API's
Browse files Browse the repository at this point in the history 
Given the unknown nature of how the system is deployed `readOnly = true` can behave in different ways.

For example with amazon aurora and the mariadb driver if you set `readOnly = true` it will send all the requests to a read
replica endpoint which is subject to lag and inconsistency. There is no serializable isolation level. While the potential
benefits here are nice it is not worth the risk of side effects both within the system codebase itself as modules are replaced
with unknown implementations or to the REST API clients who expect consistent responses (e.g. read after write from HTTP 200 responses).

Removing `readOnly = true` from all but the non-critical search API's may slow down some performance due to JPA flush and
context evaluation but the consistency gaurantees are likely worth the tradeoff for most of these queries which are point
queries anyway to index backed columns.
  • Loading branch information
@tgianos
tgianos committed Feb 9, 2022
1 parent d38f58c commit f0219f7
Showing 1 changed file with 0 additions and 32 deletions.
32 changes: 0 additions & 32 deletions ...src/main/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,6 @@ public String saveApplication(@Valid final ApplicationRequest applicationRequest
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public Application getApplication(@NotBlank final String id) throws NotFoundException {
log.debug("[getApplication] Called for {}", id);
return EntityV4DtoConverters.toV4ApplicationDto(
Expand Down Expand Up @@ -446,7 +445,6 @@ public void deleteApplication(@NotBlank final String id) throws PreconditionFail
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public Set<Command> getCommandsForApplication(
@NotBlank final String id,
@Nullable final Set<CommandStatus> statuses
Expand Down Expand Up @@ -501,7 +499,6 @@ public String saveCluster(@Valid final ClusterRequest clusterRequest) throws IdA
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public Cluster getCluster(@NotBlank final String id) throws NotFoundException {
log.debug("[getCluster] Called for {}", id);
return EntityV4DtoConverters.toV4ClusterDto(
Expand Down Expand Up @@ -812,7 +809,6 @@ public String saveCommand(@Valid final CommandRequest commandRequest) throws IdA
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public Command getCommand(@NotBlank final String id) throws NotFoundException {
log.debug("[getCommand] Called for {}", id);
return EntityV4DtoConverters.toV4CommandDto(
Expand Down Expand Up @@ -1042,7 +1038,6 @@ public void setApplicationsForCommand(
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public List<Application> getApplicationsForCommand(final String id) throws NotFoundException {
log.debug("[getApplicationsForCommand] Called for {}", id);
return this.commandRepository
Expand Down Expand Up @@ -1091,7 +1086,6 @@ public void removeApplicationForCommand(
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public Set<Cluster> getClustersForCommand(
@NotBlank final String id,
@Nullable final Set<ClusterStatus> statuses
Expand All @@ -1109,7 +1103,6 @@ public Set<Cluster> getClustersForCommand(
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public List<Criterion> getClusterCriteriaForCommand(final String id) throws NotFoundException {
log.debug("[getClusterCriteriaForCommand] Called to get cluster criteria for command {}", id);
return this.commandRepository
Expand Down Expand Up @@ -1221,7 +1214,6 @@ public void removeAllClusterCriteriaForCommand(final String id) throws NotFoundE
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public Set<Command> findCommandsMatchingCriterion(
@Valid final Criterion criterion,
final boolean addDefaultStatus
Expand Down Expand Up @@ -1319,7 +1311,6 @@ public long deleteUnusedCommands(
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
@Deprecated
public com.netflix.genie.common.dto.JobRequest getV3JobRequest(@NotBlank final String id) throws GenieException {
log.debug("[getV3JobRequest] Called with id {}", id);
Expand All @@ -1334,7 +1325,6 @@ public com.netflix.genie.common.dto.JobRequest getV3JobRequest(@NotBlank final S
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public Job getJob(@NotBlank final String id) throws GenieException {
log.debug("[getJob] Called with id {}", id);
return EntityV3DtoConverters.toJobDto(
Expand All @@ -1348,7 +1338,6 @@ public Job getJob(@NotBlank final String id) throws GenieException {
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public JobExecution getJobExecution(@NotBlank final String id) throws GenieException {
log.debug("[getJobExecution] Called with id {}", id);
return EntityV3DtoConverters.toJobExecutionDto(
Expand All @@ -1362,7 +1351,6 @@ public JobExecution getJobExecution(@NotBlank final String id) throws GenieExcep
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public com.netflix.genie.common.dto.JobMetadata getJobMetadata(@NotBlank final String id) throws GenieException {
log.debug("[getJobMetadata] Called with id {}", id);
return EntityV3DtoConverters.toJobMetadataDto(
Expand Down Expand Up @@ -1622,7 +1610,6 @@ public String saveJobSubmission(@Valid final JobSubmission jobSubmission) throws
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public JobRequest getJobRequest(@NotBlank final String id) throws NotFoundException {
log.debug("[getJobRequest] Requested for id {}", id);
return this.jobRepository
Expand Down Expand Up @@ -1700,7 +1687,6 @@ public void saveResolvedJob(
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public Optional<JobSpecification> getJobSpecification(@NotBlank final String id) throws NotFoundException {
log.debug("[getJobSpecification] Requested to get job specification for job {}", id);
final JobSpecificationProjection projection = this.jobRepository
Expand Down Expand Up @@ -1866,7 +1852,6 @@ public void updateJobArchiveStatus(
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public JobStatus getJobStatus(@NotBlank final String id) throws NotFoundException {
return DtoConverters.toV4JobStatus(
this.jobRepository
Expand All @@ -1879,7 +1864,6 @@ public JobStatus getJobStatus(@NotBlank final String id) throws NotFoundExceptio
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public ArchiveStatus getJobArchiveStatus(@NotBlank final String id) throws NotFoundException {
try {
return ArchiveStatus.valueOf(
Expand All @@ -1896,7 +1880,6 @@ public ArchiveStatus getJobArchiveStatus(@NotBlank final String id) throws NotFo
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public Optional<String> getJobArchiveLocation(@NotBlank final String id) throws NotFoundException {
final CriteriaBuilder criteriaBuilder = this.entityManager.getCriteriaBuilder();
final CriteriaQuery<String> query = criteriaBuilder.createQuery(String.class);
Expand All @@ -1918,7 +1901,6 @@ public Optional<String> getJobArchiveLocation(@NotBlank final String id) throws
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public FinishedJob getFinishedJob(@NotBlank final String id) throws NotFoundException, GenieInvalidStatusException {
// TODO
return this.jobRepository.findByUniqueId(id, FinishedJobProjection.class)
Expand All @@ -1930,7 +1912,6 @@ public FinishedJob getFinishedJob(@NotBlank final String id) throws NotFoundExce
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public boolean isApiJob(@NotBlank final String id) throws NotFoundException {
return this.jobRepository
.isAPI(id)
Expand All @@ -1941,7 +1922,6 @@ public boolean isApiJob(@NotBlank final String id) throws NotFoundException {
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public Cluster getJobCluster(@NotBlank final String id) throws NotFoundException {
log.debug("[getJobCluster] Called for job {}", id);
return EntityV4DtoConverters.toV4ClusterDto(
Expand All @@ -1956,7 +1936,6 @@ public Cluster getJobCluster(@NotBlank final String id) throws NotFoundException
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public Command getJobCommand(@NotBlank final String id) throws NotFoundException {
log.debug("[getJobCommand] Called for job {}", id);
return EntityV4DtoConverters.toV4CommandDto(
Expand All @@ -1971,7 +1950,6 @@ public Command getJobCommand(@NotBlank final String id) throws NotFoundException
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public List<Application> getJobApplications(@NotBlank final String id) throws NotFoundException {
log.debug("[getJobApplications] Called for job {}", id);
return this.jobRepository.getJobApplications(id)
Expand Down Expand Up @@ -2020,7 +1998,6 @@ public Map<String, UserResourcesSummary> getUserResourcesSummaries(
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public long getUsedMemoryOnHost(@NotBlank final String hostname) {
log.debug("[getUsedMemoryOnHost] Called for hostname {}", hostname);
return this.jobRepository.getTotalMemoryUsedOnHost(hostname, USING_MEMORY_JOB_SET);
Expand All @@ -2030,7 +2007,6 @@ public long getUsedMemoryOnHost(@NotBlank final String hostname) {
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public Set<String> getActiveJobs() {
log.debug("[getActiveJobs] Called");
return this.jobRepository.getJobIdsWithStatusIn(ACTIVE_STATUS_SET);
Expand All @@ -2040,7 +2016,6 @@ public Set<String> getActiveJobs() {
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public Set<String> getUnclaimedJobs() {
log.debug("[getUnclaimedJobs] Called");
return this.jobRepository.getJobIdsWithStatusIn(UNCLAIMED_STATUS_SET);
Expand All @@ -2050,7 +2025,6 @@ public Set<String> getUnclaimedJobs() {
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public JobInfoAggregate getHostJobInformation(@NotBlank final String hostname) {
log.debug("[getHostJobInformation] Called for hostname {}", hostname);
return this.jobRepository.getHostJobInfo(hostname, ACTIVE_STATUS_SET, USING_MEMORY_JOB_SET);
Expand All @@ -2060,7 +2034,6 @@ public JobInfoAggregate getHostJobInformation(@NotBlank final String hostname) {
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public Set<String> getJobsWithStatusAndArchiveStatusUpdatedBefore(
@NotEmpty final Set<JobStatus> statuses,
@NotEmpty final Set<ArchiveStatus> archiveStatuses,
Expand Down Expand Up @@ -2102,7 +2075,6 @@ public void updateRequestedLauncherExt(
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public JsonNode getRequestedLauncherExt(@NotBlank final String id) throws NotFoundException {
log.debug("[getRequestedLauncherExt] Requested for job {}", id);
return this.jobRepository.getRequestedLauncherExt(id).orElse(NullNode.getInstance());
Expand Down Expand Up @@ -2130,7 +2102,6 @@ public void updateLauncherExt(
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public JsonNode getLauncherExt(@NotBlank final String id) throws NotFoundException {
log.debug("[getLauncherExt] Requested for job {}", id);
return this.jobRepository.getLauncherExt(id).orElse(NullNode.getInstance());
Expand All @@ -2157,7 +2128,6 @@ public <R extends CommonResource> void addConfigsToResource(
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public <R extends CommonResource> Set<String> getConfigsForResource(
@NotBlank final String id,
final Class<R> resourceClass
Expand Down Expand Up @@ -2222,7 +2192,6 @@ public <R extends CommonResource> void addDependenciesToResource(
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public <R extends CommonResource> Set<String> getDependenciesForResource(
@NotBlank final String id,
final Class<R> resourceClass
Expand Down Expand Up @@ -2287,7 +2256,6 @@ public <R extends CommonResource> void addTagsToResource(
* {@inheritDoc}
*/
@Override
@Transactional(readOnly = true)
public <R extends CommonResource> Set<String> getTagsForResource(
@NotBlank final String id,
final Class<R> resourceClass
Expand Down

0 comments on commit f0219f7

Please sign in to comment.