From 0b83c2786ba5b376926b26d0c8689a0e4abe6246 Mon Sep 17 00:00:00 2001 From: Veronika Solovei Date: Mon, 20 May 2019 17:32:15 -0700 Subject: [PATCH 1/6] Cache validation fix if no bids returned - don't throw cache error, just return empty result --- endpoints/openrtb2/video_auction.go | 6 +++++- exchange/exchange.go | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/endpoints/openrtb2/video_auction.go b/endpoints/openrtb2/video_auction.go index acb28307534..e95ed88328b 100644 --- a/endpoints/openrtb2/video_auction.go +++ b/endpoints/openrtb2/video_auction.go @@ -359,8 +359,10 @@ func minMax(array []int) (int, int) { func buildVideoResponse(bidresponse *openrtb.BidResponse, podErrors []PodError) (*openrtb_ext.BidResponseVideo, error) { adPods := make([]*openrtb_ext.AdPod, 0) + anyBidsReturned := false for _, seatBid := range bidresponse.SeatBid { for _, bid := range seatBid.Bid { + anyBidsReturned = true var tempRespBidExt openrtb_ext.ExtBid if err := json.Unmarshal(bid.Ext, &tempRespBidExt); err != nil { @@ -393,7 +395,9 @@ func buildVideoResponse(bidresponse *openrtb.BidResponse, podErrors []PodError) } } - if len(adPods) == 0 { + //check if there are any bids in response. + //if there are no bids - empty response should be returned, no cache errors + if len(adPods) == 0 && anyBidsReturned { //means there is a global cache error, we need to reject all bids err := errors.New("caching failed for all bids") return nil, err diff --git a/exchange/exchange.go b/exchange/exchange.go index 5fd1bb6a29f..bf36db9885b 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -145,7 +145,7 @@ func (e *exchange) HoldAuction(ctx context.Context, bidRequest *openrtb.BidReque return nil, fmt.Errorf("Error in category mapping : %s", err.Error()) } - if targData != nil && adapterBids != nil { + if targData != nil && len(auc.winningBidsByBidder) > 0 { auc.setRoundedPrices(targData.priceGranularity) cacheErrs := auc.doCache(ctx, e.cache, targData, bidRequest, 60, &e.defaultTTLs, bidCategory) if len(cacheErrs) > 0 { From 3e84ad822f761bf0fdff889bc328765f589e12b1 Mon Sep 17 00:00:00 2001 From: Veronika Solovei Date: Tue, 21 May 2019 13:25:37 -0700 Subject: [PATCH 2/6] Cache validation fix - optimization: do not run auction logic if no bids returned --- exchange/exchange.go | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/exchange/exchange.go b/exchange/exchange.go index bf36db9885b..55aea330861 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -139,20 +139,30 @@ func (e *exchange) HoldAuction(ctx context.Context, bidRequest *openrtb.BidReque conversions := e.currencyConverter.Rates() adapterBids, adapterExtra := e.getAllBids(auctionCtx, cleanRequests, aliases, bidAdjustmentFactors, blabels, conversions) - bidCategory, adapterBids, err := applyCategoryMapping(ctx, requestExt, adapterBids, *categoriesFetcher, targData) - auc := newAuction(adapterBids, len(bidRequest.Imp)) - if err != nil { - return nil, fmt.Errorf("Error in category mapping : %s", err.Error()) + anyBidsReturned := false + for _, seatBid := range adapterBids { + if seatBid != nil && len(seatBid.bids) > 0 { + anyBidsReturned = true + break + } } + if anyBidsReturned { + bidCategory, adapterBids, err := applyCategoryMapping(ctx, requestExt, adapterBids, *categoriesFetcher, targData) + auc := newAuction(adapterBids, len(bidRequest.Imp)) + if err != nil { + return nil, fmt.Errorf("Error in category mapping : %s", err.Error()) + } - if targData != nil && len(auc.winningBidsByBidder) > 0 { - auc.setRoundedPrices(targData.priceGranularity) - cacheErrs := auc.doCache(ctx, e.cache, targData, bidRequest, 60, &e.defaultTTLs, bidCategory) - if len(cacheErrs) > 0 { - errs = append(errs, cacheErrs...) + if targData != nil { + auc.setRoundedPrices(targData.priceGranularity) + cacheErrs := auc.doCache(ctx, e.cache, targData, bidRequest, 60, &e.defaultTTLs, bidCategory) + if len(cacheErrs) > 0 { + errs = append(errs, cacheErrs...) + } + targData.setTargeting(auc, bidRequest.App != nil, bidCategory) } - targData.setTargeting(auc, bidRequest.App != nil, bidCategory) } + // Build the response return e.buildBidResponse(ctx, liveAdapters, adapterBids, bidRequest, resolvedRequest, adapterExtra, errs) } From 043be36b0e9a1fc340cf010ae9b86ed9348429c7 Mon Sep 17 00:00:00 2001 From: Veronika Solovei Date: Wed, 29 May 2019 16:41:49 -0700 Subject: [PATCH 3/6] Cache validation fix: minor refactoring --- exchange/exchange.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/exchange/exchange.go b/exchange/exchange.go index 55aea330861..0d23e6d85cf 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -148,11 +148,12 @@ func (e *exchange) HoldAuction(ctx context.Context, bidRequest *openrtb.BidReque } if anyBidsReturned { bidCategory, adapterBids, err := applyCategoryMapping(ctx, requestExt, adapterBids, *categoriesFetcher, targData) - auc := newAuction(adapterBids, len(bidRequest.Imp)) if err != nil { return nil, fmt.Errorf("Error in category mapping : %s", err.Error()) } + auc := newAuction(adapterBids, len(bidRequest.Imp)) + if targData != nil { auc.setRoundedPrices(targData.priceGranularity) cacheErrs := auc.doCache(ctx, e.cache, targData, bidRequest, 60, &e.defaultTTLs, bidCategory) From 6739537f9f7c6ea64249830370edffcefcf7c6c0 Mon Sep 17 00:00:00 2001 From: Veronika Solovei Date: Fri, 31 May 2019 14:41:50 -0700 Subject: [PATCH 4/6] Cache validation fix: minor refactoring --- endpoints/openrtb2/video_auction.go | 6 +----- exchange/exchange.go | 19 +++++++++---------- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/endpoints/openrtb2/video_auction.go b/endpoints/openrtb2/video_auction.go index e95ed88328b..0f5b82975e4 100644 --- a/endpoints/openrtb2/video_auction.go +++ b/endpoints/openrtb2/video_auction.go @@ -403,8 +403,6 @@ func buildVideoResponse(bidresponse *openrtb.BidResponse, podErrors []PodError) return nil, err } - videoResponse := openrtb_ext.BidResponseVideo{} - // If there were incorrect pods, we put them back to response with error message if len(podErrors) > 0 { for _, podEr := range podErrors { @@ -416,9 +414,7 @@ func buildVideoResponse(bidresponse *openrtb.BidResponse, podErrors []PodError) } } - videoResponse.AdPods = adPods - - return &videoResponse, nil + return &openrtb_ext.BidResponseVideo{AdPods: adPods}, nil } func findAdPod(podInd int64, pods []*openrtb_ext.AdPod) *openrtb_ext.AdPod { diff --git a/exchange/exchange.go b/exchange/exchange.go index 0d23e6d85cf..7cd5c495e25 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -138,14 +138,8 @@ func (e *exchange) HoldAuction(ctx context.Context, bidRequest *openrtb.BidReque // Get currency rates conversions for the auction conversions := e.currencyConverter.Rates() - adapterBids, adapterExtra := e.getAllBids(auctionCtx, cleanRequests, aliases, bidAdjustmentFactors, blabels, conversions) - anyBidsReturned := false - for _, seatBid := range adapterBids { - if seatBid != nil && len(seatBid.bids) > 0 { - anyBidsReturned = true - break - } - } + adapterBids, adapterExtra, anyBidsReturned := e.getAllBids(auctionCtx, cleanRequests, aliases, bidAdjustmentFactors, blabels, conversions) + if anyBidsReturned { bidCategory, adapterBids, err := applyCategoryMapping(ctx, requestExt, adapterBids, *categoriesFetcher, targData) if err != nil { @@ -180,11 +174,12 @@ func (e *exchange) makeAuctionContext(ctx context.Context, needsCache bool) (auc } // This piece sends all the requests to the bidder adapters and gathers the results. -func (e *exchange) getAllBids(ctx context.Context, cleanRequests map[openrtb_ext.BidderName]*openrtb.BidRequest, aliases map[string]string, bidAdjustments map[string]float64, blabels map[openrtb_ext.BidderName]*pbsmetrics.AdapterLabels, conversions currencies.Conversions) (map[openrtb_ext.BidderName]*pbsOrtbSeatBid, map[openrtb_ext.BidderName]*seatResponseExtra) { +func (e *exchange) getAllBids(ctx context.Context, cleanRequests map[openrtb_ext.BidderName]*openrtb.BidRequest, aliases map[string]string, bidAdjustments map[string]float64, blabels map[openrtb_ext.BidderName]*pbsmetrics.AdapterLabels, conversions currencies.Conversions) (map[openrtb_ext.BidderName]*pbsOrtbSeatBid, map[openrtb_ext.BidderName]*seatResponseExtra, bool) { // Set up pointers to the bid results adapterBids := make(map[openrtb_ext.BidderName]*pbsOrtbSeatBid, len(cleanRequests)) adapterExtra := make(map[openrtb_ext.BidderName]*seatResponseExtra, len(cleanRequests)) chBids := make(chan *bidResponseWrapper, len(cleanRequests)) + bidsFound := false for bidderName, req := range cleanRequests { // Here we actually call the adapters and collect the bids. @@ -239,9 +234,13 @@ func (e *exchange) getAllBids(ctx context.Context, cleanRequests map[openrtb_ext brw := <-chBids adapterBids[brw.bidder] = brw.adapterBids adapterExtra[brw.bidder] = brw.adapterExtra + + if !bidsFound && adapterBids[brw.bidder] != nil && len(adapterBids[brw.bidder].bids) > 0 { + bidsFound = true + } } - return adapterBids, adapterExtra + return adapterBids, adapterExtra, bidsFound } func (e *exchange) recoverSafely(inner func(openrtb_ext.BidderName, openrtb_ext.BidderName, *openrtb.BidRequest, *pbsmetrics.AdapterLabels, currencies.Conversions), chBids chan *bidResponseWrapper) func(openrtb_ext.BidderName, openrtb_ext.BidderName, *openrtb.BidRequest, *pbsmetrics.AdapterLabels, currencies.Conversions) { From e82b10ecfc7becf168f193d44a3ddacf11d2edde Mon Sep 17 00:00:00 2001 From: Veronika Solovei Date: Fri, 31 May 2019 18:50:06 -0700 Subject: [PATCH 5/6] Cache validation fix: added unit test for no bids returned --- endpoints/openrtb2/video_auction_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/endpoints/openrtb2/video_auction_test.go b/endpoints/openrtb2/video_auction_test.go index ab08299057a..c8bbe8d5c7f 100644 --- a/endpoints/openrtb2/video_auction_test.go +++ b/endpoints/openrtb2/video_auction_test.go @@ -478,6 +478,16 @@ func TestVideoBuildVideoResponsePodErrors(t *testing.T) { assert.Equal(t, int64(333), bidRespVideo.AdPods[2].PodId, "AdPods should contain error element at index 2") } +func TestVideoBuildVideoResponseNoBids(t *testing.T) { + openRtbBidResp := openrtb.BidResponse{} + podErrors := make([]PodError, 0, 2) + seatBids := make([]openrtb.SeatBid, 0) + openRtbBidResp.SeatBid = seatBids + bidRespVideo, err := buildVideoResponse(&openRtbBidResp, podErrors) + assert.NoError(t, err, "Error should be nil") + assert.Len(t, bidRespVideo.AdPods, 0, "AdPods length should be 0") +} + func mockDeps(t *testing.T, ex *mockExchangeVideo) *endpointDeps { theMetrics := pbsmetrics.NewMetrics(metrics.NewRegistry(), openrtb_ext.BidderList()) edep := &endpointDeps{ From abff28a7ef8f296e10002c9c6314a7dfc4144e82 Mon Sep 17 00:00:00 2001 From: Veronika Solovei Date: Mon, 3 Jun 2019 09:34:24 -0700 Subject: [PATCH 6/6] Cache validation fix: minor refactoring --- endpoints/openrtb2/video_auction_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/endpoints/openrtb2/video_auction_test.go b/endpoints/openrtb2/video_auction_test.go index c8bbe8d5c7f..8ee6609f3e8 100644 --- a/endpoints/openrtb2/video_auction_test.go +++ b/endpoints/openrtb2/video_auction_test.go @@ -480,9 +480,8 @@ func TestVideoBuildVideoResponsePodErrors(t *testing.T) { func TestVideoBuildVideoResponseNoBids(t *testing.T) { openRtbBidResp := openrtb.BidResponse{} - podErrors := make([]PodError, 0, 2) - seatBids := make([]openrtb.SeatBid, 0) - openRtbBidResp.SeatBid = seatBids + podErrors := make([]PodError, 0, 0) + openRtbBidResp.SeatBid = make([]openrtb.SeatBid, 0) bidRespVideo, err := buildVideoResponse(&openRtbBidResp, podErrors) assert.NoError(t, err, "Error should be nil") assert.Len(t, bidRespVideo.AdPods, 0, "AdPods length should be 0")