Traffic Server
  1. Traffic Server
  2. TS-1258

Need the ability to allow a user to alter the background fill config values on a per transaction basis

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 3.0.4
    • Fix Version/s: 3.3.0
    • Component/s: Cache
    • Labels:
      None

      Description

      Currently the background fill values can only be altered by the records.config file and are static for all transactions. There is no method to alter these values on a per transaction basis. This feature would be useful if we wanted to be more aggressive in caching certain types of files over others.

      Also it would be good to know if we are in a currently in a background fill ie if the client has closed the connection.

      1. TS-1258.patch
        10 kB
        Robert Logue

        Activity

        Hide
        Robert Logue added a comment -

        A patch file for TS 3.0.4 that allows the background fill config items to be altered on a per transaction basis.

        Show
        Robert Logue added a comment - A patch file for TS 3.0.4 that allows the background fill config items to be altered on a per transaction basis.
        Hide
        Leif Hedstrom added a comment -

        Robert: Any chance you can rebase your patch against trunk ? We only commit new patches onto trunk, so it makes our review much easier if you do.

        Show
        Leif Hedstrom added a comment - Robert: Any chance you can rebase your patch against trunk ? We only commit new patches onto trunk, so it makes our review much easier if you do.
        Hide
        Leif Hedstrom added a comment -

        Also, don't forget to update the InkAPITest.cc regression test with your additions.

        Show
        Leif Hedstrom added a comment - Also, don't forget to update the InkAPITest.cc regression test with your additions.
        Hide
        Leif Hedstrom added a comment -

        Oh, and the necessary additions to both InkAPI.cc and ts/ts.h.in.

        Show
        Leif Hedstrom added a comment - Oh, and the necessary additions to both InkAPI.cc and ts/ts.h.in.
        Hide
        Robert Logue added a comment -

        So get the latest snapshot from the GIT repo and use that? Also is the change in InkAPITest.cc just changing the call to TSMgmtFloatGet to a call to TSHttpTxnConfigFloatGet? I have the changes in the other two files but forgot to add them to this patch file, thanks for reminding me.

        Show
        Robert Logue added a comment - So get the latest snapshot from the GIT repo and use that? Also is the change in InkAPITest.cc just changing the call to TSMgmtFloatGet to a call to TSHttpTxnConfigFloatGet? I have the changes in the other two files but forgot to add them to this patch file, thanks for reminding me.
        Hide
        Leif Hedstrom added a comment -

        Any updates on this?

        Show
        Leif Hedstrom added a comment - Any updates on this?
        Hide
        Robert Logue added a comment -

        I was wondering what codeline to use, or rather where to get the trunk from? I was just waiting on a response to that. If you can point me in the right direction I will create the patch from that codeline.

        Show
        Robert Logue added a comment - I was wondering what codeline to use, or rather where to get the trunk from? I was just waiting on a response to that. If you can point me in the right direction I will create the patch from that codeline.
        Hide
        Leif Hedstrom added a comment -

        Oh. Yeah, that's documented on our web site, http://trafficserver.apache.org. But alas, it's

        $ git clone https://git-wip-us.apache.org/repos/asf/trafficserver.git
        

        As for making sure regressions etc. are right, look at one of the other "float" configs (there's only one or two) that are overridable, and make sure you do everything that is done for those.

        Show
        Leif Hedstrom added a comment - Oh. Yeah, that's documented on our web site, http://trafficserver.apache.org . But alas, it's $ git clone https: //git-wip-us.apache.org/repos/asf/trafficserver.git As for making sure regressions etc. are right, look at one of the other "float" configs (there's only one or two) that are overridable, and make sure you do everything that is done for those.
        Hide
        Robert Logue added a comment - - edited

        Updated patch file with missing files and regression test update. If anything else is needed for the regression test please let me know as I ma not too familar with the running of these.

        Show
        Robert Logue added a comment - - edited Updated patch file with missing files and regression test update. If anything else is needed for the regression test please let me know as I ma not too familar with the running of these.
        Hide
        James Peach added a comment -

        Leif, can you review and commit?

        Show
        James Peach added a comment - Leif, can you review and commit?
        Hide
        Leif Hedstrom added a comment -

        Looks good, except I'm going to change the new API to:

        +int
        +TSHttpTxnBackgroundFillStarted(TSHttpTxn txnp)
        +{
        +  sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
        +  HttpSM *s = (HttpSM*) txnp;
        +
        +  return (s->background_fill == BACKGROUND_FILL_STARTED);
        +}
        +
        
        Show
        Leif Hedstrom added a comment - Looks good, except I'm going to change the new API to: + int +TSHttpTxnBackgroundFillStarted(TSHttpTxn txnp) +{ + sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS); + HttpSM *s = (HttpSM*) txnp; + + return (s->background_fill == BACKGROUND_FILL_STARTED); +} +
        Hide
        Leif Hedstrom added a comment -

        Actually, need to do a few additional changes to the patch, I'll try to get it done later tonight or tomorrow. The new enum's should have different names, and at least one of the default is changing its value in the patch, which I want to examine.

        Show
        Leif Hedstrom added a comment - Actually, need to do a few additional changes to the patch, I'll try to get it done later tonight or tomorrow. The new enum's should have different names, and at least one of the default is changing its value in the patch, which I want to examine.
        Hide
        Leif Hedstrom added a comment -

        Fixed in 6b929347a16a773e86b670a2a48a575a3d15e38d.

        Show
        Leif Hedstrom added a comment - Fixed in 6b929347a16a773e86b670a2a48a575a3d15e38d.

          People

          • Assignee:
            Leif Hedstrom
            Reporter:
            Robert Logue
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development