Skip to content

Commit

Permalink
Transfer files - AQL optimizations
Browse files Browse the repository at this point in the history
  • Loading branch information
yahavi committed Nov 19, 2023
1 parent 52da35c commit 2c1812c
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 10 deletions.
40 changes: 40 additions & 0 deletions artifactory/commands/transferfiles/filediff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,43 @@ func TestConvertResultsToFileRepresentation(t *testing.T) {
assert.Equal(t, []api.FileRepresentation{testCase.expectedOutput}, files)
}
}

var generateDiffAqlQueryTestCases = []struct {
paginationOffset int
disabledDistinctAql bool
expectedAql string
}{
{0, false, "items.find({\"$and\":[{\"modified\":{\"$gte\":\"1\"}},{\"modified\":{\"$lt\":\"2\"}},{\"repo\":\"repo1\",\"type\":\"any\"}]}).include(\"repo\",\"path\",\"name\",\"type\",\"modified\",\"size\").sort({\"$asc\":[\"name\",\"path\"]}).offset(0).limit(10000)"},
{0, true, "items.find({\"$and\":[{\"modified\":{\"$gte\":\"1\"}},{\"modified\":{\"$lt\":\"2\"}},{\"repo\":\"repo1\",\"type\":\"any\"}]}).include(\"repo\",\"path\",\"name\",\"type\",\"modified\",\"size\").sort({\"$asc\":[\"name\",\"path\"]}).offset(0).limit(10000).distinct(false)"},
{2, false, "items.find({\"$and\":[{\"modified\":{\"$gte\":\"1\"}},{\"modified\":{\"$lt\":\"2\"}},{\"repo\":\"repo1\",\"type\":\"any\"}]}).include(\"repo\",\"path\",\"name\",\"type\",\"modified\",\"size\").sort({\"$asc\":[\"name\",\"path\"]}).offset(20000).limit(10000)"},
{2, true, "items.find({\"$and\":[{\"modified\":{\"$gte\":\"1\"}},{\"modified\":{\"$lt\":\"2\"}},{\"repo\":\"repo1\",\"type\":\"any\"}]}).include(\"repo\",\"path\",\"name\",\"type\",\"modified\",\"size\").sort({\"$asc\":[\"name\",\"path\"]}).offset(20000).limit(10000).distinct(false)"},
}

func TestGenerateDiffAqlQuery(t *testing.T) {
for _, testCase := range generateDiffAqlQueryTestCases {
t.Run("", func(*testing.T) {
results := generateDiffAqlQuery(repo1Key, "1", "2", testCase.paginationOffset, testCase.disabledDistinctAql)
assert.Equal(t, testCase.expectedAql, results)
})
}
}

var generateDockerManifestAqlQueryTestCases = []struct {
paginationOffset int
disabledDistinctAql bool
expectedAql string
}{
{0, false, "items.find({\"$and\":[{\"repo\":\"repo1\"},{\"modified\":{\"$gte\":\"1\"}},{\"modified\":{\"$lt\":\"2\"}},{\"$or\":[{\"name\":\"manifest.json\"},{\"name\":\"list.manifest.json\"}]}]}).include(\"repo\",\"path\",\"name\",\"type\",\"modified\").sort({\"$asc\":[\"name\",\"path\"]}).offset(0).limit(10000)"},
{0, true, "items.find({\"$and\":[{\"repo\":\"repo1\"},{\"modified\":{\"$gte\":\"1\"}},{\"modified\":{\"$lt\":\"2\"}},{\"$or\":[{\"name\":\"manifest.json\"},{\"name\":\"list.manifest.json\"}]}]}).include(\"repo\",\"path\",\"name\",\"type\",\"modified\").sort({\"$asc\":[\"name\",\"path\"]}).offset(0).limit(10000).distinct(false)"},
{2, false, "items.find({\"$and\":[{\"repo\":\"repo1\"},{\"modified\":{\"$gte\":\"1\"}},{\"modified\":{\"$lt\":\"2\"}},{\"$or\":[{\"name\":\"manifest.json\"},{\"name\":\"list.manifest.json\"}]}]}).include(\"repo\",\"path\",\"name\",\"type\",\"modified\").sort({\"$asc\":[\"name\",\"path\"]}).offset(20000).limit(10000)"},
{2, true, "items.find({\"$and\":[{\"repo\":\"repo1\"},{\"modified\":{\"$gte\":\"1\"}},{\"modified\":{\"$lt\":\"2\"}},{\"$or\":[{\"name\":\"manifest.json\"},{\"name\":\"list.manifest.json\"}]}]}).include(\"repo\",\"path\",\"name\",\"type\",\"modified\").sort({\"$asc\":[\"name\",\"path\"]}).offset(20000).limit(10000).distinct(false)"},
}

func TestGenerateDockerManifestAqlQuery(t *testing.T) {
for _, testCase := range generateDockerManifestAqlQueryTestCases {
t.Run("", func(*testing.T) {
results := generateDockerManifestAqlQuery(repo1Key, "1", "2", testCase.paginationOffset, testCase.disabledDistinctAql)
assert.Equal(t, testCase.expectedAql, results)
})
}
}
22 changes: 14 additions & 8 deletions artifactory/commands/transferfiles/filesdiff.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func (f *filesDiffPhase) getTimeFrameFilesDiff(fromTimestamp, toTimestamp string
}

func (f *filesDiffPhase) getNonDockerTimeFrameFilesDiff(fromTimestamp, toTimestamp string, paginationOffset int) (aqlResult *servicesUtils.AqlSearchResult, err error) {
query := generateDiffAqlQuery(f.repoKey, fromTimestamp, toTimestamp, paginationOffset)
query := generateDiffAqlQuery(f.repoKey, fromTimestamp, toTimestamp, paginationOffset, f.disabledDistinctAql)
return runAql(f.context, f.srcRtDetails, query)
}

Expand All @@ -225,7 +225,7 @@ func (f *filesDiffPhase) getNonDockerTimeFrameFilesDiff(fromTimestamp, toTimesta
// to get all artifacts in its path (that includes the "manifest.json" file itself and all its layouts).
func (f *filesDiffPhase) getDockerTimeFrameFilesDiff(fromTimestamp, toTimestamp string, paginationOffset int) (aqlResult *servicesUtils.AqlSearchResult, err error) {
// Get all newly created or modified manifest files ("manifest.json" and "list.manifest.json" files)
query := generateDockerManifestAqlQuery(f.repoKey, fromTimestamp, toTimestamp, paginationOffset)
query := generateDockerManifestAqlQuery(f.repoKey, fromTimestamp, toTimestamp, paginationOffset, f.disabledDistinctAql)
manifestFilesResult, err := runAql(f.context, f.srcRtDetails, query)
if err != nil {
return
Expand Down Expand Up @@ -261,11 +261,10 @@ func (f *filesDiffPhase) getDockerTimeFrameFilesDiff(fromTimestamp, toTimestamp
return
}

func generateDiffAqlQuery(repoKey, fromTimestamp, toTimestamp string, paginationOffset int) string {
func generateDiffAqlQuery(repoKey, fromTimestamp, toTimestamp string, paginationOffset int, disabledDistinctAql bool) string {
query := fmt.Sprintf(`items.find({"$and":[{"modified":{"$gte":"%s"}},{"modified":{"$lt":"%s"}},{"repo":"%s","type":"any"}]})`, fromTimestamp, toTimestamp, repoKey)
query += `.include("repo","path","name","type","modified","size")`
query += fmt.Sprintf(`.sort({"$asc":["modified"]}).offset(%d).limit(%d)`, paginationOffset*AqlPaginationLimit, AqlPaginationLimit)
return query
return query + generateAqlSortingPart(paginationOffset, disabledDistinctAql)
}

// This function generates an AQL that searches for all the content in the list of provided Artifactory paths.
Expand All @@ -283,10 +282,17 @@ func generateGetDirContentAqlQuery(repoKey string, paths []string) string {
}

// This function generates an AQL that searches for all files named "manifest.json" and "list.manifest.json" in a specific repository.
func generateDockerManifestAqlQuery(repoKey, fromTimestamp, toTimestamp string, paginationOffset int) string {
func generateDockerManifestAqlQuery(repoKey, fromTimestamp, toTimestamp string, paginationOffset int, disabledDistinctAql bool) string {
query := `items.find({"$and":`
query += fmt.Sprintf(`[{"repo":"%s"},{"modified":{"$gte":"%s"}},{"modified":{"$lt":"%s"}},{"$or":[{"name":"manifest.json"},{"name":"list.manifest.json"}]}`, repoKey, fromTimestamp, toTimestamp)
query += `]}).include("repo","path","name","type","modified")`
query += fmt.Sprintf(`.sort({"$asc":["modified"]}).offset(%d).limit(%d)`, paginationOffset*AqlPaginationLimit, AqlPaginationLimit)
return query
return query + generateAqlSortingPart(paginationOffset, disabledDistinctAql)
}

func generateAqlSortingPart(paginationOffset int, disabledDistinctAql bool) string {
sortingPart := fmt.Sprintf(`.sort({"$asc":["name","path"]}).offset(%d).limit(%d)`, paginationOffset*AqlPaginationLimit, AqlPaginationLimit)
if disabledDistinctAql {
sortingPart += `.distinct(false)`
}
return sortingPart
}
7 changes: 5 additions & 2 deletions artifactory/commands/transferfiles/fulltransfer.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ func getFolderRelativePath(folderName, relativeLocation string) string {
}

func (m *fullTransferPhase) getDirectoryContentAql(relativePath string, paginationOffset int) (result []servicesUtils.ResultItem, lastPage bool, err error) {
query := generateFolderContentAqlQuery(m.repoKey, relativePath, paginationOffset)
query := generateFolderContentAqlQuery(m.repoKey, relativePath, paginationOffset, m.disabledDistinctAql)
aqlResults, err := runAql(m.context, m.srcRtDetails, query)
if err != nil {
return []servicesUtils.ResultItem{}, false, err
Expand All @@ -284,10 +284,13 @@ func (m *fullTransferPhase) getDirectoryContentAql(relativePath string, paginati
return
}

func generateFolderContentAqlQuery(repoKey, relativePath string, paginationOffset int) string {
func generateFolderContentAqlQuery(repoKey, relativePath string, paginationOffset int, disabledDistinctAql bool) string {
query := fmt.Sprintf(`items.find({"type":"any","$or":[{"$and":[{"repo":"%s","path":{"$match":"%s"},"name":{"$match":"*"}}]}]})`, repoKey, relativePath)
query += `.include("repo","path","name","type","size")`
query += fmt.Sprintf(`.sort({"$asc":["name"]}).offset(%d).limit(%d)`, paginationOffset*AqlPaginationLimit, AqlPaginationLimit)
if disabledDistinctAql {
query += `.distinct(false)`
}
return query
}

Expand Down
7 changes: 7 additions & 0 deletions artifactory/commands/transferfiles/phase.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ type transferPhase interface {
setProxyKey(proxyKey string)
setBuildInfo(setBuildInfo bool)
setPackageType(packageType string)
setDisabledDistinctAql()
setStopSignal(stopSignal chan os.Signal)
StopGracefully()
}
Expand All @@ -59,6 +60,8 @@ type phaseBase struct {
stateManager *state.TransferStateManager
locallyGeneratedFilter *locallyGeneratedFilter
stopSignal chan os.Signal
// Optimization in Artifactory version 7.37 and above enables the exclusion of setting DISTINCT in SQL queries
disabledDistinctAql bool
}

func (pb *phaseBase) ShouldStop() bool {
Expand Down Expand Up @@ -140,6 +143,10 @@ func (pb *phaseBase) setPackageType(packageType string) {
pb.packageType = packageType
}

func (pb *phaseBase) setDisabledDistinctAql() {
pb.disabledDistinctAql = true
}

func (pb *phaseBase) setStopSignal(stopSignal chan os.Signal) {
pb.stopSignal = stopSignal
}
Expand Down
33 changes: 33 additions & 0 deletions artifactory/commands/transferfiles/transfer.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"syscall"
"time"

"github.com/jfrog/gofrog/version"
"github.com/jfrog/jfrog-cli-core/v2/artifactory/commands/transferfiles/state"
"github.com/jfrog/jfrog-cli-core/v2/artifactory/commands/utils/precheckrunner"
"github.com/jfrog/jfrog-cli-core/v2/artifactory/utils"
Expand All @@ -33,6 +34,7 @@ const (
retries = 600
retriesWaitMilliSecs = 5000
dataTransferPluginMinVersion = "1.7.0"
disableDistinctAqlMinVersion = "7.37"
)

type TransferFilesCommand struct {
Expand All @@ -55,6 +57,8 @@ type TransferFilesCommand struct {
stateManager *state.TransferStateManager
preChecks bool
locallyGeneratedFilter *locallyGeneratedFilter
// Optimization in Artifactory version 7.37 and above enables the exclusion of setting DISTINCT in SQL queries
disabledDistinctAql bool
}

func NewTransferFilesCommand(sourceServer, targetServer *config.ServerDetails) (*TransferFilesCommand, error) {
Expand Down Expand Up @@ -182,6 +186,10 @@ func (tdc *TransferFilesCommand) Run() (err error) {
return err
}

if err = tdc.initDistinctAql(); err != nil {
return err
}

if err = tdc.initStateManager(allSourceLocalRepos, sourceBuildInfoRepos); err != nil {
return err
}
Expand Down Expand Up @@ -291,6 +299,28 @@ func (tdc *TransferFilesCommand) initStorageInfoManagers() error {
return storageInfoManager.CalculateStorageInfo()
}

func (tdc *TransferFilesCommand) initDistinctAql() error {
// Init source storage services manager
servicesManager, err := createTransferServiceManager(tdc.context, tdc.sourceServerDetails)
if err != nil {
return err
}

// Getting source Artifactory version
sourceArtifactoryVersion, err := servicesManager.GetVersion()
if err != nil {
return err
}

// If version is at least 7.37, add .distinct(false) to AQL queries
if version.NewVersion(sourceArtifactoryVersion).AtLeast(disableDistinctAqlMinVersion) {
tdc.disabledDistinctAql = true
log.Debug(fmt.Sprintf("The source Artifactory version is above %s (%s). Adding .distinct(false) to AQL requests.",
disableDistinctAqlMinVersion, sourceArtifactoryVersion))
}
return nil
}

// Creates the Pre-checks runner for the data transfer command
func (tdc *TransferFilesCommand) NewTransferDataPreChecksRunner() (runner *precheckrunner.PreCheckRunner, err error) {
// Get relevant repos
Expand Down Expand Up @@ -458,6 +488,9 @@ func (tdc *TransferFilesCommand) startPhase(newPhase *transferPhase, repo string
if err != nil {
return err
}
if tdc.disabledDistinctAql {
(*newPhase).setDisabledDistinctAql()
}
printPhaseChange("Running '" + (*newPhase).getPhaseName() + "' for repo '" + repo + "'...")
err = (*newPhase).run()
if err != nil {
Expand Down

0 comments on commit 2c1812c

Please sign in to comment.