Skip to content

Commit

Permalink
Merge branch 'master' into plugin_interface
Browse files Browse the repository at this point in the history
  • Loading branch information
Mantas Šidlauskas authored Apr 24, 2024
2 parents 5d0f3a4 + 44b1f59 commit 4e1ab9e
Show file tree
Hide file tree
Showing 9 changed files with 1,061 additions and 175 deletions.
1 change: 1 addition & 0 deletions common/client/versionChecker.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ func (vc *versionChecker) SupportsStickyQuery(clientImpl string, clientFeatureVe
// SupportsConsistentQuery returns error if consistent query is not supported otherwise nil.
// In case client version lookup fails assume the client does not support feature.
func (vc *versionChecker) SupportsConsistentQuery(clientImpl string, clientFeatureVersion string) error {

return vc.featureSupported(clientImpl, clientFeatureVersion, consistentQuery)
}

Expand Down
9 changes: 7 additions & 2 deletions common/pinot/pinotQueryValidator.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,8 +437,13 @@ func parseCloseStatus(original *sqlparser.SQLVal) (*sqlparser.SQLVal, error) {
statusStr := string(original.Val)

// first check if already in int64 format
if _, err := strconv.ParseInt(statusStr, 10, 64); err == nil {
return original, nil
if status, err := strconv.ParseInt(statusStr, 10, 64); err == nil {
// Instead of returning the original value, return a new SQLVal that holds the integer value
// Or it will fail the case CloseStatus = '1'
return &sqlparser.SQLVal{
Type: sqlparser.IntVal,
Val: []byte(strconv.FormatInt(status, 10)),
}, nil
}

// try to parse close status string
Expand Down
11 changes: 9 additions & 2 deletions common/pinot/pinotQueryValidator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,15 +219,22 @@ func TestValidateQuery(t *testing.T) {
query: "CustomKeywordField < 0",
validated: "(JSON_MATCH(Attr, '\"$.CustomKeywordField\"<''0''') or JSON_MATCH(Attr, '\"$.CustomKeywordField[*]\"<''0'''))",
},
// TODO
"Case18: custom int order by. Will have errors at run time. Doesn't support for now": {
query: "CustomIntField = 0 order by CustomIntField desc",
validated: "JSON_MATCH(Attr, '\"$.CustomIntField\"=''0''') order by CustomIntField desc",
},
"case 19: close status parse": {
"case19-1: close status parse string": {
query: "CloseStatus = 'CONTINUED_AS_NEW'",
validated: "CloseStatus = 4",
},
"case19-2: close status parse number": {
query: "CloseStatus = '1'",
validated: "CloseStatus = 1",
},
"case19-3: close status parse normal case": {
query: "CloseStatus = 1",
validated: "CloseStatus = 1",
},
}

for name, test := range tests {
Expand Down
14 changes: 12 additions & 2 deletions service/frontend/wrappers/clusterredirection/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,12 @@ func (policy *selectedOrAllAPIsForwardingRedirectionPolicy) WithDomainIDRedirect
return err
}
if domainEntry.IsDeprecatedOrDeleted() {
return fmt.Errorf("domain %v is deprecated or deleted", domainEntry.GetInfo().Name)
return types.DomainNotActiveError{
Message: "domain is deprecated.",
DomainName: domainEntry.GetInfo().Name,
CurrentCluster: policy.currentClusterName,
ActiveCluster: policy.currentClusterName,
}
}
return policy.withRedirect(ctx, domainEntry, apiName, call)
}
Expand All @@ -207,7 +212,12 @@ func (policy *selectedOrAllAPIsForwardingRedirectionPolicy) WithDomainNameRedire
return err
}
if domainEntry.IsDeprecatedOrDeleted() {
return fmt.Errorf("domain %v is deprecated or deleted", domainName)
return types.DomainNotActiveError{
Message: "domain is deprecated or deleted",
DomainName: domainName,
CurrentCluster: policy.currentClusterName,
ActiveCluster: policy.currentClusterName,
}
}
return policy.withRedirect(ctx, domainEntry, apiName, call)
}
Expand Down
4 changes: 2 additions & 2 deletions service/frontend/wrappers/clusterredirection/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,11 +398,11 @@ func (s *selectedAPIsForwardingRedirectionPolicySuite) TestGetTargetDataCenter_G
for apiName := range selectedAPIsForwardingRedirectionPolicyAPIAllowlist {
err := s.policy.WithDomainIDRedirect(context.Background(), s.domainID, apiName, callFn)
s.Error(err)
s.Equal(fmt.Sprintf("domain %v is deprecated or deleted", s.domainName), err.Error())
s.Equal("domain is deprecated.", err.Error())

err = s.policy.WithDomainNameRedirect(context.Background(), s.domainName, apiName, callFn)
s.Error(err)
s.Equal(fmt.Sprintf("domain %v is deprecated or deleted", s.domainName), err.Error())
s.Equal("domain is deprecated or deleted", err.Error())
}

s.Equal(0, currentClustercallCount)
Expand Down
10 changes: 9 additions & 1 deletion service/history/decision/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,15 @@ func (handler *handlerImpl) handleBufferedQueries(

// Consistent query requires both server and client worker support. If a consistent query was requested (meaning there are
// buffered queries) but worker does not support consistent query then all buffered queries should be failed.
if versionErr := handler.versionChecker.SupportsConsistentQuery(clientImpl, clientFeatureVersion); versionErr != nil {
versionErr := handler.versionChecker.SupportsConsistentQuery(clientImpl, clientFeatureVersion)
// todo (David.Porter) remove the skip on version check for
// clientImpl and clientFeatureVersion where they're nil
// There's a bug, probably in matching somewhere which isn't
// forwarding the client headers for version
// info correctly making this call erroneously fail sometimes.
// https://t3.uberinternal.com/browse/CDNC-8641
// So defaulting just this flow to fail-open in the absence of headers.
if versionErr != nil && clientImpl != "" && clientFeatureVersion != "" {
scope.IncCounter(metrics.WorkerNotSupportsConsistentQueryCount)
failedTerminationState := &query.TerminationState{
TerminationType: query.TerminationTypeFailed,
Expand Down
Loading

0 comments on commit 4e1ab9e

Please sign in to comment.