If the final goal is to make this as the server level configuration (and remove the table level in the future), I recommend adding this config inside of `StreamConfig` because it will be much easier to remove the config from StreamConfig because it's a map.
As Subbu mentioned, we may need to put it in the table config if this feature needs to work for both offline/realtime. @Ting Is this feature will only be used for realtime? On Mon, May 11, 2020 at 11:10 AM Subbu Subramaniam <[email protected]> wrote: > The goal of the config change in [arts other than StreamConfig is to > ensure that the config can be used for offline segments as well, if for > some reason download fails. > > However, a race condition can happen for offline segments that can be > dangerous. If a segment has been updated with a newer version, and server A > and B have old versions. Both of them get notified of the newer version. > They may try to fetch the segment and fail, and eventually fetch from each > other, and end up thinking that they have the newest version of the > segment.There can be other variants of this as well, with restarts of > server. > > I can think of some ways to fix this (e.g. in the segment update message, > send the crc of the new version), but these have not been fully thought of. > We need to vet these well before adopting these. > > I prefer option 1 since it introduces a single config. > > I would like to hear from @kishoreg and @npawar as well > > -Subbu > > On 2020/05/06 00:16:26, TING CHEN <[email protected]> wrote: > > As part of the proposal > > < > https://cwiki.apache.org/confluence/display/PINOT/By-passing+deep-store+requirement+for+Realtime+segment+completion > > > > to bypass deep store for segment completion, I plan to add a new optional > > string field *peerSegmentDownloadScheme* to > > the SegmentsValidationAndRetentionConfig in the TableConfig. The value > can > > be *http* or *https*. > > > > 1. SplitSegmentCommitter > > < > https://github.com/apache/incubator-pinot/blob/31c55afdb6a40f98189308ce6292587ead9d0dec/pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/SplitSegmentCommitter.java > > > > will check this value. If it exists, the segment committer will be > able to > > finish segment commit successfully even if the upload to the segment > store > > fails. The committer will report a special marker to the controller > about > > the segment is available in peer servers. > > 2. When Pinot servers fail to download segments from the segment > store, > > they can also check this field's value. If it exists, it can download > > segments from peer servers using either HTTP and HTTPS segment > fetchers as > > configured. (related PR > > <https://github.com/apache/incubator-pinot/pull/5336> in review for > how > > to discover such servers.) > > > > Note this is a table level config. We will test the new download behavior > > in realtime tables in incremental fashion. Once fully proven, this config > > can be upgraded to server level config. > > > > Please let me know if you have any questions on this. Thanks @mcvsubbu > for > > coming up with the idea and offline discussions. > > > > Ting Chen > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
