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.

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

[WIP] Multi-Bid Support #2254

Closed

Conversation

pm-nilesh-chate
Copy link
Contributor

@pm-nilesh-chate pm-nilesh-chate commented May 22, 2022

Multi-Bid Support #1715
#1715

@pm-nilesh-chate pm-nilesh-chate changed the title [draft] Multi-Bid Support #1715 [draft] Multi-Bid Support May 22, 2022
@pm-nilesh-chate
Copy link
Contributor Author

pm-nilesh-chate commented May 22, 2022

Hi @bretg,

I'm working on multi-bid support for Go-PBS and have started these code changes in reference to Java's multi-bid implementation at prebid/prebid-server-java#1169.

I needed help on a few questions before I continue with the changes.

  • From Java-PBS, what I see is for winning-bid logic, winningBidsByBidder now will have all the bids for an impression rather than only one winning bid per impression.
  • Is it safe to discard all the bids from an impression that were not selected by sort and maxBids limit. If correct, shouldn't all the bids under each impression qualify as winningBidsByBidder candidate? making winningBidsByBidder field obsolete.
  • Existing Go-PBS does not assert 1 bid per impression, do we make it max 1 bid per impression if multi-bid is not support

Note: This is a draft POC, will complete the feature, clean the code, etc before converting it to PR.

@pm-nilesh-chate pm-nilesh-chate changed the title [draft] Multi-Bid Support [WIP] Multi-Bid Support Jun 21, 2022
@mansinahar
Copy link
Contributor

@bretg Can you please help @pm-nilesh-chate with his questions above when you have a chance? Thanks!

@bretg
Copy link
Contributor

bretg commented Jul 19, 2022

The first two questions refer to a specific coding detail 'winningBidsByBidder' that I do not understand. I'm going to ignore that, and instead just restate what I believe is already stated in #1715 :

  1. by default, PBS returns all seat bids to the client
  2. by default, targeting is only applied to the top bid from each seat (assuming includebidderkeys is set)
  3. the multibid feature applies targeting to bids beyond the top bid from each seat (assuming includebidderkeys is set)
  4. if multibid is not set and if host/account config auction.default-bid-limit-min is set, PBS will constrain the number bids. (Note: that config name is kinda confusing, but that's what PBS-Java has called it for a long time. Think of it as the 'bid-limit-that-can-be-modified-upwards-by-multibid')

Existing Go-PBS does not assert 1 bid per impression, do we make it max 1 bid per impression if multi-bid is not support

The purpose of this 'auction.default-bid-limit-min' setting is to bring PBS-Go/Java together on this behavior. It's why Req 15 was added -- #1715 (comment)

@pm-nilesh-chate
Copy link
Contributor Author

pm-nilesh-chate commented Nov 25, 2022

Refer #2468 for further updates.
Continuing the changes with the correct GitHub account. Apologies for the inconvenience.

@pm-nilesh-chate pm-nilesh-chate deleted the NYC-multi-bid-1 branch November 25, 2022 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants