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

S3 Search Improvements #1391

Merged
merged 26 commits into from Jan 31, 2017

Conversation

Projects
None yet
3 participants
@ssalinas
Member

ssalinas commented Jan 4, 2017

Opening this early for any feedback.

Currently, all data about which files to upload to s3, and any possible bucket or key pattern overrides is stored in the executor configuration. This unfortunately means that SingularityService has no idea if it needs to search a separate bucket/pattern to find logs. It can only search what it knows about from it's defaults and the request group overrides.

Notable changes so far:

  • s3 file and file name related configurations are moved from the executor to the service. This way it is available for searching and/or ui use more easily
  • loggingS3Bucket removed from ExecutorData on the deploy. We have overrides by group and by file name for this purpose. If we want a request level override separate from the group one we can add that later
  • The executor data passed to the custom executor is now the SingularityTaskExecutorData object not the ExecutorData from the deploy. This way we can pass additional information to the custom executor from system-wide configuration.
  • S3LogResource will check the now available s3 additional files to determine if additional buckets need to be searched

Still TODO:

  • Optionally skip get/download url generation to improve efficiency of the logs endpoints
  • Generate proper prefixes for key pattern overrides
  • Add request group as possible param in the uploader key pattern
  • Paginate the s3 search endpoint
  • Docs for config changes

Note - SingularityService needs to be deployed before the executor for this particular change

/cc @tpetr

@ssalinas ssalinas changed the title from (WIP) Move S3 file configuration to SingularityService to Move S3 file configuration to SingularityService Jan 5, 2017

ssalinas added some commits Jan 6, 2017

@ssalinas

This comment has been minimized.

Show comment
Hide comment
@ssalinas

ssalinas Jan 6, 2017

Member

FYI, now building this on top of the commits from #1375 . It modifies/moves a bunch of the same files and the merges are a bit horrid otherwise

Member

ssalinas commented Jan 6, 2017

FYI, now building this on top of the commits from #1375 . It modifies/moves a bunch of the same files and the merges are a bit horrid otherwise

@ssalinas ssalinas added the hs_staging label Jan 6, 2017

@ssalinas ssalinas changed the title from Move S3 file configuration to SingularityService to S3 Search Improvements Jan 9, 2017

ssalinas added some commits Jan 9, 2017

@ssalinas ssalinas added the hs_qa label Jan 10, 2017

ssalinas added some commits Jan 10, 2017

Implement pagination for S3 endpoint using continuation tokens
need @Consumes on POST endpoints

more s3 pagination tweaks

more attempts at better pagination

need >= here

fix maxPerPage

more robust search options

add missing file

fix for findbugs

continuation token format needs group too

use isTruncated for ending

re-request to respect page size

revert re-request for page size

missing tokens gives false positive end of content

typo
Merge pull request #1395 from HubSpot/s3_pagination
Implement pagination for S3 endpoint using continuation tokens

@ssalinas ssalinas added this to the 0.14.0 milestone Jan 11, 2017

@ssalinas ssalinas added the hs_stable label Jan 18, 2017

@tpetr

This comment has been minimized.

Show comment
Hide comment
@tpetr

tpetr Jan 19, 2017

Member

🚢

Member

tpetr commented on 95d40fd Jan 19, 2017

🚢

@tpetr

This comment has been minimized.

Show comment
Hide comment
@tpetr

tpetr Jan 19, 2017

Member

🚢

Member

tpetr commented on 6811cb0 Jan 19, 2017

🚢

@tpetr

This comment has been minimized.

Show comment
Hide comment
@tpetr

tpetr Jan 19, 2017

Member

🚢

Member

tpetr commented on 13d2fe9 Jan 19, 2017

🚢

@tpetr

This comment has been minimized.

Show comment
Hide comment
@tpetr

tpetr Jan 19, 2017

Member

🚢

Member

tpetr commented on e26e8dc Jan 19, 2017

🚢

@tpetr

This comment has been minimized.

Show comment
Hide comment
@tpetr

tpetr Jan 19, 2017

Member

🚢

Member

tpetr commented on 57f3ffe Jan 19, 2017

🚢

ssalinas added some commits Jan 20, 2017

@ssalinas

This comment has been minimized.

Show comment
Hide comment
@ssalinas

ssalinas Jan 31, 2017

Member

Added the docs for this now, going to merge

Member

ssalinas commented Jan 31, 2017

Added the docs for this now, going to merge

@ssalinas ssalinas merged commit cfbab9c into master Jan 31, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@ssalinas ssalinas deleted the s3_rework branch Jan 31, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment