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.

Sign up for GitHub

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

Jump to bottom

Transfer - AQL optimizations #1036

Merged
merged 2 commits into from
Nov 20, 2023
Merged

Transfer - AQL optimizations #1036

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a39438d
Transfer - AQL optimizations
yahavi Nov 19, 2023
9f6e83c
Merge branch 'dev' into aql-optimizations
yahavi Nov 20, 2023
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
40 changes: 40 additions & 0 deletions artifactory/commands/transferfiles/filediff_test.go
View file
Open in desktop
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
disabledDistinctiveAql 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.disabledDistinctiveAql)
assert.Equal(t, testCase.expectedAql, results)
})
}
}

var generateDockerManifestAqlQueryTestCases = []struct {
paginationOffset int
disabledDistinctiveAql 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.disabledDistinctiveAql)
assert.Equal(t, testCase.expectedAql, results)
})
}
}
22 changes: 14 additions & 8 deletions artifactory/commands/transferfiles/filesdiff.go
View file
Open in desktop
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.disabledDistinctiveAql)
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.disabledDistinctiveAql)
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, disabledDistinctiveAql 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, disabledDistinctiveAql)
}

// 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, disabledDistinctiveAql 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, disabledDistinctiveAql)
}

func generateAqlSortingPart(paginationOffset int, disabledDistinctiveAql bool) string {
sortingPart := fmt.Sprintf(`.sort({"$asc":["name","path"]}).offset(%d).limit(%d)`, paginationOffset*AqlPaginationLimit, AqlPaginationLimit)
if disabledDistinctiveAql {
sortingPart += `.distinct(false)`
}
return sortingPart
}
7 changes: 5 additions & 2 deletions artifactory/commands/transferfiles/fulltransfer.go
View file
Open in desktop
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.disabledDistinctiveAql)
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, disabledDistinctiveAql 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 disabledDistinctiveAql {
query += `.distinct(false)`
}
return query
}

Expand Down
7 changes: 7 additions & 0 deletions artifactory/commands/transferfiles/phase.go
View file
Open in desktop
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)
setDisabledDistinctiveAql()
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
disabledDistinctiveAql 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) setDisabledDistinctiveAql() {
pb.disabledDistinctiveAql = 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
View file
Open in desktop
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
disabledDistinctiveAql 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.disabledDistinctiveAql = 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.disabledDistinctiveAql {
(*newPhase).setDisabledDistinctiveAql()
}
printPhaseChange("Running '" + (*newPhase).getPhaseName() + "' for repo '" + repo + "'...")
err = (*newPhase).run()
if err != nil {
Expand Down
Loading