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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filter getRequestHistory queries by createdAt times #2117

Merged
merged 4 commits into from
Jul 17, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
package com.hubspot.singularity;

import com.google.common.base.Predicate;
import com.google.common.collect.ComparisonChain;
import java.util.Comparator;
import java.util.Optional;

public class SingularityRequestHistoryQuery {
private final String requestId;
private final Optional<Long> createdBefore;
private final Optional<Long> createdAfter;
private final Optional<OrderDirection> orderDirection;

public SingularityRequestHistoryQuery(
String requestId,
Optional<Long> createdBefore,
Optional<Long> createdAfter,
Optional<OrderDirection> orderDirection
) {
this.requestId = requestId;
this.createdBefore = createdBefore;
this.createdAfter = createdAfter;
this.orderDirection = orderDirection;
}

public String getRequestId() {
return requestId;
}

public Optional<Long> getCreatedBefore() {
return createdBefore;
}

public Optional<Long> getCreatedAfter() {
return createdAfter;
}

public Optional<OrderDirection> getOrderDirection() {
return orderDirection;
}

public Predicate<SingularityRequestHistory> getHistoryFilter() {
return new Predicate<SingularityRequestHistory>() {

@Override
public boolean apply(SingularityRequestHistory input) {
if (!requestId.equals(input.getRequest().getId())) {
return false;
}

if (createdAfter.isPresent() && createdAfter.get() >= input.getCreatedAt()) {
return false;
}

if (createdBefore.isPresent() && createdBefore.get() <= input.getCreatedAt()) {
return false;
}

return true;
}
};
}

public Comparator<SingularityRequestHistory> getComparator() {
final OrderDirection localOrderDirection = orderDirection.orElse(OrderDirection.DESC);

return new Comparator<SingularityRequestHistory>() {

@Override
public int compare(SingularityRequestHistory o1, SingularityRequestHistory o2) {
ComparisonChain chain = ComparisonChain.start();

if (localOrderDirection == OrderDirection.ASC) {
chain = chain.compare(o1.getCreatedAt(), o2.getCreatedAt());
} else {
chain = chain.compare(o2.getCreatedAt(), o1.getCreatedAt());
}

return chain.compare(o1.getRequest().getId(), o2.getRequest().getId()).result();
}
};
}

@Override
public String toString() {
return (
"SingularityRequestHistoryQuery{" +
"requestId=" +
requestId +
", createdBefore=" +
createdBefore +
", createdAfter=" +
createdAfter +
", orderDirection=" +
orderDirection +
'}'
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import com.hubspot.singularity.SingularityRequestCleanup;
import com.hubspot.singularity.SingularityRequestGroup;
import com.hubspot.singularity.SingularityRequestHistory;
import com.hubspot.singularity.SingularityRequestHistoryQuery;
import com.hubspot.singularity.SingularityRequestParent;
import com.hubspot.singularity.SingularityRequestWithState;
import com.hubspot.singularity.SingularityS3Log;
Expand Down Expand Up @@ -1669,6 +1670,10 @@ public void deleteSlave(String slaveId) {
*
* @param requestId
* Request ID to look up
* @param createdBefore
* Long millis to filter request histories created before
* @param createdAfter
* Long millis to filter request histories created after
* @param count
* Number of items to return per page
* @param page
Expand All @@ -1678,6 +1683,8 @@ public void deleteSlave(String slaveId) {
*/
public Collection<SingularityRequestHistory> getHistoryForRequest(
String requestId,
Optional<Long> createdBefore,
Optional<Long> createdAfter,
Optional<Integer> count,
Optional<Integer> page
) {
Expand All @@ -1688,6 +1695,14 @@ public Collection<SingularityRequestHistory> getHistoryForRequest(

ImmutableMap.Builder<String, Object> queryParamsBuilder = ImmutableMap.builder();

if (createdBefore.isPresent()) {
queryParamsBuilder.put("createdBefore", createdBefore.get());
}

if (createdAfter.isPresent()) {
queryParamsBuilder.put("createdAfter", createdAfter.get());
}

if (count.isPresent()) {
queryParamsBuilder.put("count", count.get());
}
Expand All @@ -1709,6 +1724,20 @@ public Collection<SingularityRequestHistory> getHistoryForRequest(
);
}

public Collection<SingularityRequestHistory> getHistoryForRequest(
String requestId,
Optional<Integer> count,
Optional<Integer> page
) {
return getHistoryForRequest(
requestId,
Optional.empty(),
Optional.empty(),
count,
page
);
}

//
// Inactive/Bad Slaves
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

import com.hubspot.singularity.ExtendedTaskState;
import com.hubspot.singularity.OrderDirection;
import com.hubspot.singularity.SingularityRequestHistory;
import com.hubspot.singularity.SingularityTaskIdHistory;
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
Expand All @@ -28,6 +30,32 @@ default void addWhereOrAnd(StringBuilder sqlBuilder, boolean shouldUseWhere) {
}
}

String getRequestHistoryBaseQuery();

default void applyRequestHistoryBaseQuery(
StringBuilder sqlBuilder,
Map<String, Object> binds,
String requestId,
Optional<Long> createdBefore,
Optional<Long> createdAfter
) {
addWhereOrAnd(sqlBuilder, binds.isEmpty());
sqlBuilder.append("requestId = :requestId");
binds.put("requestId", requestId);

if (createdBefore.isPresent()) {
addWhereOrAnd(sqlBuilder, binds.isEmpty());
sqlBuilder.append("createdAt < :createdBefore");
binds.put("createdBefore", new Date(createdBefore.get()));
}

if (createdAfter.isPresent()) {
addWhereOrAnd(sqlBuilder, binds.isEmpty());
sqlBuilder.append("createdAt > :createdAfter");
binds.put("createdAfter", new Date(createdAfter.get()));
}
}

default void applyTaskIdHistoryBaseQuery(
StringBuilder sqlBuilder,
Map<String, Object> binds,
Expand Down Expand Up @@ -156,6 +184,47 @@ default List<SingularityTaskIdHistory> getTaskIdHistory(
return query.mapTo(SingularityTaskIdHistory.class).list();
}

default List<SingularityRequestHistory> getRequestHistory(
String requestId,
Optional<Long> createdBefore,
Optional<Long> createdAfter,
String orderDirection,
Integer limitStart,
Integer limitCount
) {
final Map<String, Object> binds = new HashMap<>();
final StringBuilder sqlBuilder = new StringBuilder(getRequestHistoryBaseQuery());

applyRequestHistoryBaseQuery(
sqlBuilder,
binds,
requestId,
createdBefore,
createdAfter
);

sqlBuilder.append(" ORDER BY createdAt ");
sqlBuilder.append(orderDirection);
if (limitCount != null) {
sqlBuilder.append(" LIMIT :limitCount");
binds.put("limitCount", limitCount);
}

if (limitStart != null) {
sqlBuilder.append(" OFFSET :limitStart ");
binds.put("limitStart", limitStart);
}

final String sql = sqlBuilder.toString();

LOG.trace("Generated sql for request history search: {}, binds: {}", sql, binds);

Query query = getHandle().createQuery(sql);
binds.forEach(query::bind);

return query.mapTo(SingularityRequestHistory.class).list();
}

default int getTaskIdHistoryCount(
Optional<String> requestId,
Optional<String> deployId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ List<SingularityDeployHistory> getDeployHistoryForRequest(

List<SingularityRequestHistory> getRequestHistory(
String requestId,
Optional<Long> createdBefore,
Optional<Long> createdAfter,
String orderDirection,
Integer limitStart,
Integer limitCount
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ int getTaskIdHistoryCount(

List<SingularityRequestHistory> getRequestHistory(
String requestId,
Optional<Long> createdBefore,
Optional<Long> createdAfter,
Optional<OrderDirection> orderDirection,
Integer limitStart,
Integer limitCount
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,20 +271,26 @@ private String getOrderDirection(Optional<OrderDirection> orderDirection) {
@Override
public List<SingularityRequestHistory> getRequestHistory(
String requestId,
Optional<Long> createdBefore,
Optional<Long> createdAfter,
Optional<OrderDirection> orderDirection,
Integer limitStart,
Integer limitCount
) {
List<SingularityRequestHistory> singularityRequestHistoryList = history.getRequestHistory(
requestId,
createdBefore,
createdAfter,
getOrderDirection(orderDirection),
limitStart,
limitCount
);
if (LOG.isTraceEnabled()) {
LOG.trace(
"getRequestHistory requestId {}, orderDirection {}, limitStart {} , limitCount {}, requestHistory{}",
"getRequestHistory requestId {}, createdBefore {}, createdAfter {}, orderDirection {}, limitStart {} , limitCount {}, requestHistory{}",
requestId,
createdBefore,
createdAfter,
orderDirection,
limitStart,
limitCount,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,13 @@

import com.hubspot.singularity.SingularityDeployHistory;
import com.hubspot.singularity.SingularityRequest;
import com.hubspot.singularity.SingularityRequestHistory;
import com.hubspot.singularity.SingularityTaskHistory;
import com.hubspot.singularity.data.history.SingularityMappers.SingularityRequestIdCount;
import java.util.Date;
import java.util.List;
import org.jdbi.v3.json.Json;
import org.jdbi.v3.sqlobject.SingleValue;
import org.jdbi.v3.sqlobject.customizer.Bind;
import org.jdbi.v3.sqlobject.customizer.Define;
import org.jdbi.v3.sqlobject.statement.SqlQuery;
import org.jdbi.v3.sqlobject.statement.SqlUpdate;

Expand Down Expand Up @@ -93,16 +91,6 @@ List<SingularityDeployHistory> getDeployHistoryForRequest(
@SqlQuery("SELECT COUNT(*) FROM deployHistory WHERE requestId = :requestId")
int getDeployHistoryForRequestCount(@Bind("requestId") String requestId);

@SqlQuery(
"SELECT json, request, createdAt, requestState, user, message FROM requestHistory WHERE requestId = :requestId ORDER BY createdAt <orderDirection> LIMIT :limitStart, :limitCount"
)
List<SingularityRequestHistory> getRequestHistory(
@Bind("requestId") String requestId,
@Define("orderDirection") String orderDirection,
@Bind("limitStart") Integer limitStart,
@Bind("limitCount") Integer limitCount
);

@SqlQuery("SELECT COUNT(*) FROM requestHistory WHERE requestId = :requestId")
int getRequestHistoryCount(@Bind("requestId") String requestId);

Expand Down Expand Up @@ -253,5 +241,10 @@ void setDeployJson(
@Bind("json") @Json SingularityDeployHistory deployHistory
);

@Override
default String getRequestHistoryBaseQuery() {
return "SELECT json, request, createdAt, requestState, user, message FROM requestHistory";
}

default void close() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ public Optional<SingularityTaskHistory> getTaskHistoryByRunId(
@Override
public List<SingularityRequestHistory> getRequestHistory(
String requestId,
Optional<Long> createdBefore,
Optional<Long> createdAfter,
Optional<OrderDirection> orderDirection,
Integer limitStart,
Integer limitCount
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,13 @@

import com.hubspot.singularity.SingularityDeployHistory;
import com.hubspot.singularity.SingularityRequest;
import com.hubspot.singularity.SingularityRequestHistory;
import com.hubspot.singularity.SingularityTaskHistory;
import com.hubspot.singularity.data.history.SingularityMappers.SingularityRequestIdCount;
import java.util.Date;
import java.util.List;
import org.jdbi.v3.json.Json;
import org.jdbi.v3.sqlobject.SingleValue;
import org.jdbi.v3.sqlobject.customizer.Bind;
import org.jdbi.v3.sqlobject.customizer.Define;
import org.jdbi.v3.sqlobject.statement.SqlQuery;
import org.jdbi.v3.sqlobject.statement.SqlUpdate;

Expand Down Expand Up @@ -93,16 +91,6 @@ List<SingularityDeployHistory> getDeployHistoryForRequest(
@SqlQuery("SELECT COUNT(*) FROM deployHistory WHERE requestId = :requestId")
int getDeployHistoryForRequestCount(@Bind("requestId") String requestId);

@SqlQuery(
"SELECT json, request, createdAt, requestState, f_user, message FROM requestHistory WHERE requestId = :requestId ORDER BY createdAt <orderDirection> OFFSET :limitStart LIMIT :limitCount"
)
List<SingularityRequestHistory> getRequestHistory(
@Bind("requestId") String requestId,
@Define("orderDirection") String orderDirection,
@Bind("limitStart") Integer limitStart,
@Bind("limitCount") Integer limitCount
);

@SqlQuery("SELECT COUNT(*) FROM requestHistory WHERE requestId = :requestId")
int getRequestHistoryCount(@Bind("requestId") String requestId);

Expand Down Expand Up @@ -253,5 +241,10 @@ void setDeployJson(
@Bind("json") @Json SingularityDeployHistory deployHistory
);

@Override
default String getRequestHistoryBaseQuery() {
Copy link
Member

Choose a reason for hiding this comment

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

If this is just a constant let's declare it as a string on the class, not as a method, same for the mysql jdbi version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The user column is named user in the mysql version, and f_user in the postgres version. That's why I didn't declare it as a constant.

Copy link
Member

Choose a reason for hiding this comment

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

ah, totally read over that difference. It can still just be a String REQUEST_HISTORY_BASE_QUERY = "..."; on each class though, vs a method

Copy link
Member

Choose a reason for hiding this comment

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

sry, read that again, ignore me...

return "SELECT json, request, createdAt, requestState, f_user, message FROM requestHistory";
}

default void close() {}
}
Loading